[PATCH 1 of 3 V3] mdiff: add a "blocksinrange" function to filter diff blocks by line range
FUJIWARA Katsunori
foozy at lares.dti.ne.jp
Tue Jan 3 14:56:17 UTC 2017
I confirmed that this series resolves the issue, which I metioned at
previous post. Thanks !
Some nit picks below.
At Fri, 30 Dec 2016 15:01:29 +0100,
Denis Laxalde wrote:
>
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde at logilab.fr>
> # Date 1476279051 -7200
> # Wed Oct 12 15:30:51 2016 +0200
> # Node ID 17eee60ccbafe46f539a75484ebf110377781fb1
> # Parent 342d0cb4f446826169a83a6e773f1c767e0c977b
> # EXP-Topic linerange-log/revset
> mdiff: add a "blocksinrange" function to filter diff blocks by line range
>
> The function filters diff blocks as generated by mdiff.allblock function based
> on whether they are contained in a given line range based on the "b-side" of
> blocks.
>
(snip)
> diff --git a/tests/test-linerange.py b/tests/test-linerange.py
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-linerange.py
> @@ -0,0 +1,117 @@
> +from __future__ import absolute_import
> +
> +import unittest
> +from mercurial import error, mdiff
> +
> +def filteredblocks(blocks, rangeb):
> + """return `rangea` extracted from `blocks` coming from
> + `mdiff.blocksinrange` along with the mask of blocks within rangeb.
> + """
> + filtered, rangea = mdiff.blocksinrange(blocks, rangeb)
> + skipped = [b not in filtered for b in blocks]
> + return rangea, skipped
> +
> +class blocksinrangetests(unittest.TestCase):
> +
> + def setUp(self):
> + self.blocks = [
> + ((0, 3, 0, 2), '!'),
> + ((3, 7, 2, 6), '='),
> + ((7, 12, 6, 12), '!'),
> + ((15, 15, 12, 12), '='),
> + ]
- the last element in self.bloks should be "((12, 12, 12, 12), '=')"
- to avoid this kind of mistake, how about initializing self.blocks
by mdiff.allblocks() like below ?
IMHO, this is more readable for future reviewers than tuples of
magic numbers.
================
# for readability, line numbers are 0-origin
text1 = '''
00 at OLD
01 at OLD
02 at OLD
02 at NEW, 03 at OLD
03 at NEW, 04 at OLD
04 at NEW, 05 at OLD
05 at NEW, 06 at OLD
07 at OLD
08 at OLD
09 at OLD
10 at OLD
11 at OLD
'''[1:] # strip initial LF
text2 = '''
00 at NEW
01 at NEW
02 at NEW, 03 at OLD
03 at NEW, 04 at OLD
04 at NEW, 05 at OLD
05 at NEW, 06 at OLD
06 at NEW
07 at NEW
08 at NEW
09 at NEW
10 at NEW
11 at NEW
'''[1:] # strip initial LF
....
def setUp(self):
self.blocks = mdiff.allblocks(text1, text2)
================
> + def testWithinEqual(self):
> + linerange2 = (3, 5)
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (4, 6))
> + self.assertEqual(skipped, [True, False, True, True])
As for natives, is name of these tests enough for understanding what
it examines ?
How about the comment like below for readability ?
> +
> + def testWithinEqualStrictly(self):
# IDX 0 1
# 012345678901
# SRC NNOOOONNNNNN (New/Old)
# ^^^^
> + linerange2 = (2, 6)
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (3, 7))
> + self.assertEqual(skipped, [True, False, True, True])
> +
> + def testWithinEqualLowerboundOneline(self):
> + linerange2 = (2, 3)
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (3, 4))
> + self.assertEqual(skipped, [True, False, True, True])
> +
> + def testWithinEqualLowerbound(self):
> + linerange2 = (2, 3)
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (3, 4))
> + self.assertEqual(skipped, [True, False, True, True])
Are testWithinEqualLowerboundOneline and testWithinEqualLowerbound
identical intentionally ?
> + def testWithinEqualUpperbound(self):
> + linerange2 = (3, 6)
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (4, 7))
> + self.assertEqual(skipped, [True, False, True, True])
Isn't "OneLine" of testWithinEqualUpperbound needed for coverage, like
as testWithinEqualLowerbound and "OneLine" of it ?
(I'm not sure whether it is certainly necessary or not)
> + def testWithinFirstBlockNeq(self):
# IDX 0 1
# 012345678901
# SRC NNOOOONNNNNN (New/Old)
# ^
# | (empty?)
# ^
# ^^
> + for linerange2 in [
> + (0, 1),
> + (1, 1),
> + (1, 2),
> + (0, 2),
> + ]:
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (0, 3))
> + self.assertEqual(skipped, [False, True, True, True])
> +
> + def testWithinLastBlockNeq(self):
# IDX 0 1
# 012345678901
# SRC NNOOOONNNNNN (New/Old)
# ^
# ^
# | (empty?)
# ^^^^^^
> + for linerange2 in [
> + (6, 7),
> + (7, 8),
> + (7, 7),
> + (6, 12),
> + ]:
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (7, 12))
> + self.assertEqual(skipped, [True, True, False, True])
> +
> + def testWithinLastLine(self):
> + linerange2 = (11, 12)
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (7, 12))
> + self.assertEqual(skipped, [True, True, False, True])
> +
> + def testAccrossTwoBlocks(self):
> + linerange2 = (1, 7)
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (0, 12))
> + self.assertEqual(skipped, [False, False, False, True])
> +
> + def testCrossingSeveralBlocks(self):
> + linerange2 = (1, 8)
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (0, 12))
> + self.assertEqual(skipped, [False, False, False, True])
> +
> + def testStartInEqBlock(self):
> + linerange2 = (5, 9)
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (6, 12))
> + self.assertEqual(skipped, [True, False, False, True])
> +
> + def testEndInEqBlock(self):
> + linerange2 = (1, 3)
> + linerange1, skipped = filteredblocks(self.blocks, linerange2)
> + self.assertEqual(linerange1, (0, 4))
> + self.assertEqual(skipped, [False, False, True, True])
> +
> + def testOutOfRange(self):
> + for linerange2 in [
> + (0, 34),
> + (15, 12),
> + ]:
> + with self.assertRaises(error.Abort) as cm:
> + mdiff.blocksinrange(self.blocks, linerange2)
> + self.assertIn('line range exceeds file size', str(cm.exception))
With Python 2.6.x, this "with self.assertRaises()" causes
unintentional failure below:
Traceback (most recent call last):
File "REPO/hg/tests/test-linerange.py", line 111, in testOutOfRange
with self.assertRaises(error.Abort) as cm:
TypeError: failUnlessRaises() takes at least 3 arguments (2 given)
Unfortunately, assertRaises() of Python 2.6.x neither allows to omit
"callable" argument, nor provide the way to examine raised exception.
Therefore, we should write try/except clause manually, for portable
implementation to check both exception raising and 'line range exceeds
file size'-ness of raised exception :-<
> +if __name__ == '__main__':
> + import silenttestrunner
> + silenttestrunner.main(__name__)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
--
----------------------------------------------------------------------
[FUJIWARA Katsunori] foozy at lares.dti.ne.jp
More information about the Mercurial-devel
mailing list