Submitter | Augie Fackler |
---|---|
Date | Dec. 15, 2016, 4:32 p.m. |
Message ID | <774639bc892b7576a53a.1481819557@arthedain.pit.corp.google.com> |
Download | mbox | patch |
Permalink | /patch/17925/ |
State | Accepted |
Headers | show |
Comments
On 12/15/2016 05:32 PM, Augie Fackler wrote: > # HG changeset patch > # User Augie Fackler <augie@google.com> > # Date 1481818440 18000 > # Thu Dec 15 11:14:00 2016 -0500 > # Node ID 774639bc892b7576a53a672dd4a4492a536f8f0f > # Parent afa152bbdf097667c095f054d78463bf6d8a666a > tests: fix test-bdiff to handle variance between pure and c bdiff code > > Obviously we'd rather patch pure to have the same algorithmic win as > the C code, but this is a quick fix for the pure build since pure > isn't wrong, just not as fast as it could be. Okay, the test upgrade looks good², and I would take it if except for the import checker complaining about the first patch. I've sent a fix for the import checker¹: https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-December/091525.html I'll push this series once that checker fix is in, unless someone else does it before me. > diff --git a/tests/test-bdiff.py b/tests/test-bdiff.py > --- a/tests/test-bdiff.py > +++ b/tests/test-bdiff.py > @@ -81,18 +81,28 @@ class BdiffTests(unittest.TestCase): > 'x\n\n', > diffreplace(9, 9, '', 'y\n\n'), > 'x\n\nz\n']), > - # we should pick up abbbc. rather than bc.de as the longest match > - ("a\nb\nb\nb\nc\n.\nd\ne\n.\nf\n", > - "a\nb\nb\na\nb\nb\nb\nc\n.\nb\nc\n.\nd\ne\nf\n", > - ['a\nb\nb\n', > - diffreplace(6, 6, '', 'a\nb\nb\nb\nc\n.\n'), > - 'b\nc\n.\nd\ne\n', > - diffreplace(16, 18, '.\n', ''), > - 'f\n']), > ] > for old, new, want in cases: > self.assertEqual(self.showdiff(old, new), want) > > + def test_issue1295_varies_on_pure(self): > + # we should pick up abbbc. rather than bc.de as the longest match > + got = self.showdiff("a\nb\nb\nb\nc\n.\nd\ne\n.\nf\n", > + "a\nb\nb\na\nb\nb\nb\nc\n.\nb\nc\n.\nd\ne\nf\n") > + want_c = ['a\nb\nb\n', > + diffreplace(6, 6, '', 'a\nb\nb\nb\nc\n.\n'), > + 'b\nc\n.\nd\ne\n', > + diffreplace(16, 18, '.\n', ''), > + 'f\n'] > + want_pure = [diffreplace(0, 0, '', 'a\nb\nb\n'), > + 'a\nb\nb\nb\nc\n.\n', > + diffreplace(12, 12, '', 'b\nc\n.\n'), > + 'd\ne\n', > + diffreplace(16, 18, '.\n', ''), 'f\n'] > + self.assert_(got in (want_c, want_pure), > + 'got: %r, wanted either %r or %r' % ( > + got, want_c, want_pure)) > + > def test_fixws(self): > cases = [ > (" \ta\r b\t\n", "ab\n", 1), > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> On Dec 15, 2016, at 3:38 PM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: > > > > On 12/15/2016 05:32 PM, Augie Fackler wrote: >> # HG changeset patch >> # User Augie Fackler <augie@google.com> >> # Date 1481818440 18000 >> # Thu Dec 15 11:14:00 2016 -0500 >> # Node ID 774639bc892b7576a53a672dd4a4492a536f8f0f >> # Parent afa152bbdf097667c095f054d78463bf6d8a666a >> tests: fix test-bdiff to handle variance between pure and c bdiff code >> >> Obviously we'd rather patch pure to have the same algorithmic win as >> the C code, but this is a quick fix for the pure build since pure >> isn't wrong, just not as fast as it could be. > > Okay, the test upgrade looks good², and I would take it if except for the import checker complaining about the first patch. I've sent a fix for the import checker¹: > > https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-December/091525.html > > I'll push this series once that checker fix is in, unless someone else does it before me. > >> diff --git a/tests/test-bdiff.py b/tests/test-bdiff.py >> --- a/tests/test-bdiff.py >> +++ b/tests/test-bdiff.py >> @@ -81,18 +81,28 @@ class BdiffTests(unittest.TestCase): >> 'x\n\n', >> diffreplace(9, 9, '', 'y\n\n'), >> 'x\n\nz\n']), >> - # we should pick up abbbc. rather than bc.de as the longest match >> - ("a\nb\nb\nb\nc\n.\nd\ne\n.\nf\n", >> - "a\nb\nb\na\nb\nb\nb\nc\n.\nb\nc\n.\nd\ne\nf\n", >> - ['a\nb\nb\n', >> - diffreplace(6, 6, '', 'a\nb\nb\nb\nc\n.\n'), >> - 'b\nc\n.\nd\ne\n', >> - diffreplace(16, 18, '.\n', ''), >> - 'f\n']), >> ] >> for old, new, want in cases: >> self.assertEqual(self.showdiff(old, new), want) >> >> + def test_issue1295_varies_on_pure(self): >> + # we should pick up abbbc. rather than bc.de as the longest match >> + got = self.showdiff("a\nb\nb\nb\nc\n.\nd\ne\n.\nf\n", >> + "a\nb\nb\na\nb\nb\nb\nc\n.\nb\nc\n.\nd\ne\nf\n") >> + want_c = ['a\nb\nb\n', >> + diffreplace(6, 6, '', 'a\nb\nb\nb\nc\n.\n'), >> + 'b\nc\n.\nd\ne\n', >> + diffreplace(16, 18, '.\n', ''), >> + 'f\n'] >> + want_pure = [diffreplace(0, 0, '', 'a\nb\nb\n'), >> + 'a\nb\nb\nb\nc\n.\n', >> + diffreplace(12, 12, '', 'b\nc\n.\n'), >> + 'd\ne\n', >> + diffreplace(16, 18, '.\n', ''), 'f\n'] >> + self.assert_(got in (want_c, want_pure), >> + 'got: %r, wanted either %r or %r' % ( >> + got, want_c, want_pure)) >> + >> def test_fixws(self): >> cases = [ >> (" \ta\r b\t\n", "ab\n", 1), >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@mercurial-scm.org >> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > > > -- > Pierre-Yves David > > [1] You don't seems to be in front a screen anymore so falling back to the normal process. > > [2] (If I had done it, would have probably tested for "pure" and pinned the result, but this is good enough and fixes the test.) I wasn’t sure offhand how to test for pure-mode, and I finished the series while in meetings, so I just did something that I knew would work. It’d be fine with me to push an alternate version of patch 5 with the refactoring in 1-4 done. Up to you though!
Patch
diff --git a/tests/test-bdiff.py b/tests/test-bdiff.py --- a/tests/test-bdiff.py +++ b/tests/test-bdiff.py @@ -81,18 +81,28 @@ class BdiffTests(unittest.TestCase): 'x\n\n', diffreplace(9, 9, '', 'y\n\n'), 'x\n\nz\n']), - # we should pick up abbbc. rather than bc.de as the longest match - ("a\nb\nb\nb\nc\n.\nd\ne\n.\nf\n", - "a\nb\nb\na\nb\nb\nb\nc\n.\nb\nc\n.\nd\ne\nf\n", - ['a\nb\nb\n', - diffreplace(6, 6, '', 'a\nb\nb\nb\nc\n.\n'), - 'b\nc\n.\nd\ne\n', - diffreplace(16, 18, '.\n', ''), - 'f\n']), ] for old, new, want in cases: self.assertEqual(self.showdiff(old, new), want) + def test_issue1295_varies_on_pure(self): + # we should pick up abbbc. rather than bc.de as the longest match + got = self.showdiff("a\nb\nb\nb\nc\n.\nd\ne\n.\nf\n", + "a\nb\nb\na\nb\nb\nb\nc\n.\nb\nc\n.\nd\ne\nf\n") + want_c = ['a\nb\nb\n', + diffreplace(6, 6, '', 'a\nb\nb\nb\nc\n.\n'), + 'b\nc\n.\nd\ne\n', + diffreplace(16, 18, '.\n', ''), + 'f\n'] + want_pure = [diffreplace(0, 0, '', 'a\nb\nb\n'), + 'a\nb\nb\nb\nc\n.\n', + diffreplace(12, 12, '', 'b\nc\n.\n'), + 'd\ne\n', + diffreplace(16, 18, '.\n', ''), 'f\n'] + self.assert_(got in (want_c, want_pure), + 'got: %r, wanted either %r or %r' % ( + got, want_c, want_pure)) + def test_fixws(self): cases = [ (" \ta\r b\t\n", "ab\n", 1),