| Submitter | via Mercurial-devel |
|---|---|
| Date | Jan. 11, 2017, 8:58 p.m. |
| Message ID | <193e2d6cba6f294dca88.1484168333@martinvonz.mtv.corp.google.com> |
| Download | mbox | patch |
| Permalink | /patch/18183/ |
| State | Changes Requested |
| Headers | show |
Comments
On Wed, Jan 11, 2017 at 12:58 PM, Martin von Zweigbergk via Mercurial-devel <mercurial-devel@mercurial-scm.org> wrote: > # HG changeset patch > # User Martin von Zweigbergk <martinvonz@google.com> > # Date 1484168228 28800 > # Wed Jan 11 12:57:08 2017 -0800 > # Node ID 193e2d6cba6f294dca88b939a2a1afa5874a9794 > # Parent 9823e2f50a935f6170e01235b65b5282680ebdab > tests: fix import order in test-bdiff > > Without this, I see the following failure in > test-check-module-imports.t. > > @@ -180,3 +180,5 @@ > > -X tests/test-hgweb-no-request-uri.t \ > > -X tests/test-hgweb-non-interactive.t \ > > | sed 's-\\-/-g' | python "$import_checker" - > + tests/test-bdiff.py:6: imports not lexically sorted: silenttestrunner < > unittest > + [1] > > diff -r 9823e2f50a93 -r 193e2d6cba6f tests/test-bdiff.py > --- a/tests/test-bdiff.py Sun Jan 08 00:52:54 2017 +0800 > +++ b/tests/test-bdiff.py Wed Jan 11 12:57:08 2017 -0800 > @@ -1,10 +1,9 @@ > from __future__ import absolute_import, print_function > import collections > +import silenttestrunner > import struct > import unittest > > -import silenttestrunner > - > from mercurial import ( > bdiff, > mpatch, > Hmmm. This is seemingly a bug in the import checker because silenttestrunner is not part of the Python standard library. But, uh, I can't reproduce this failure. If I had to take a guess, it would be that you have a silenttestrunner module installed in your Python install and that is confusing the import checker into believing it is part of the stdlib. Does `import silenttestrunner` work from a Python REPL on your machine?
On Wed, Jan 11, 2017 at 4:48 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote: > On Wed, Jan 11, 2017 at 12:58 PM, Martin von Zweigbergk via Mercurial-devel > <mercurial-devel@mercurial-scm.org> wrote: >> >> # HG changeset patch >> # User Martin von Zweigbergk <martinvonz@google.com> >> # Date 1484168228 28800 >> # Wed Jan 11 12:57:08 2017 -0800 >> # Node ID 193e2d6cba6f294dca88b939a2a1afa5874a9794 >> # Parent 9823e2f50a935f6170e01235b65b5282680ebdab >> tests: fix import order in test-bdiff >> >> Without this, I see the following failure in >> test-check-module-imports.t. >> >> @@ -180,3 +180,5 @@ >> > -X tests/test-hgweb-no-request-uri.t \ >> > -X tests/test-hgweb-non-interactive.t \ >> > | sed 's-\\-/-g' | python "$import_checker" - >> + tests/test-bdiff.py:6: imports not lexically sorted: silenttestrunner < >> unittest >> + [1] >> >> diff -r 9823e2f50a93 -r 193e2d6cba6f tests/test-bdiff.py >> --- a/tests/test-bdiff.py Sun Jan 08 00:52:54 2017 +0800 >> +++ b/tests/test-bdiff.py Wed Jan 11 12:57:08 2017 -0800 >> @@ -1,10 +1,9 @@ >> from __future__ import absolute_import, print_function >> import collections >> +import silenttestrunner >> import struct >> import unittest >> >> -import silenttestrunner >> - >> from mercurial import ( >> bdiff, >> mpatch, > > > Hmmm. This is seemingly a bug in the import checker because silenttestrunner > is not part of the Python standard library. So test-verify-repo-operations.py, test-manifest.py and test-lock.py are also incorrect in that case? > > But, uh, I can't reproduce this failure. If I had to take a guess, it would > be that you have a silenttestrunner module installed in your Python install > and that is confusing the import checker into believing it is part of the > stdlib. Does `import silenttestrunner` work from a Python REPL on your > machine? Nope, it fails.
On Wed, Jan 11, 2017 at 5:36 PM, Martin von Zweigbergk < martinvonz@google.com> wrote: > On Wed, Jan 11, 2017 at 4:48 PM, Gregory Szorc <gregory.szorc@gmail.com> > wrote: > > On Wed, Jan 11, 2017 at 12:58 PM, Martin von Zweigbergk via > Mercurial-devel > > <mercurial-devel@mercurial-scm.org> wrote: > >> > >> # HG changeset patch > >> # User Martin von Zweigbergk <martinvonz@google.com> > >> # Date 1484168228 28800 > >> # Wed Jan 11 12:57:08 2017 -0800 > >> # Node ID 193e2d6cba6f294dca88b939a2a1afa5874a9794 > >> # Parent 9823e2f50a935f6170e01235b65b5282680ebdab > >> tests: fix import order in test-bdiff > >> > >> Without this, I see the following failure in > >> test-check-module-imports.t. > >> > >> @@ -180,3 +180,5 @@ > >> > -X tests/test-hgweb-no-request-uri.t \ > >> > -X tests/test-hgweb-non-interactive.t \ > >> > | sed 's-\\-/-g' | python "$import_checker" - > >> + tests/test-bdiff.py:6: imports not lexically sorted: > silenttestrunner < > >> unittest > >> + [1] > >> > >> diff -r 9823e2f50a93 -r 193e2d6cba6f tests/test-bdiff.py > >> --- a/tests/test-bdiff.py Sun Jan 08 00:52:54 2017 +0800 > >> +++ b/tests/test-bdiff.py Wed Jan 11 12:57:08 2017 -0800 > >> @@ -1,10 +1,9 @@ > >> from __future__ import absolute_import, print_function > >> import collections > >> +import silenttestrunner > >> import struct > >> import unittest > >> > >> -import silenttestrunner > >> - > >> from mercurial import ( > >> bdiff, > >> mpatch, > > > > > > Hmmm. This is seemingly a bug in the import checker because > silenttestrunner > > is not part of the Python standard library. > > So test-verify-repo-operations.py, test-manifest.py and test-lock.py > are also incorrect in that case? > They look incorrect to me w.r.t. silenttestrunner being in the stdlib block. test-verify-repo-operations.py is also not even close to using our import convention, which is why it is excluded by test-check-module-imports.t. > > > > > But, uh, I can't reproduce this failure. If I had to take a guess, it > would > > be that you have a silenttestrunner module installed in your Python > install > > and that is confusing the import checker into believing it is part of the > > stdlib. Does `import silenttestrunner` work from a Python REPL on your > > machine? > > Nope, it fails. > I still can't reproduce your failure. If I run the import checker manually, I do get a different failure: $ echo tests/test-bdiff.py | python contrib/import-checker.py - tests/test-bdiff.py:8: direct symbol import bdiff, mpatch from mercurial And if I run a command inspired from test-check-module-imports.t: $ hg locate 'set:**.py or grep(r"^#!.*?python")' 'tests/**.t' | python contrib/import-checker.py - I don't see a failure for test-bdiff.py. Wat. I have a hunch import-checker.py is doing something wonky when processing multiple files.
On Thu, Jan 12, 2017 at 8:59 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote: > On Wed, Jan 11, 2017 at 5:36 PM, Martin von Zweigbergk < > martinvonz@google.com> wrote: > >> On Wed, Jan 11, 2017 at 4:48 PM, Gregory Szorc <gregory.szorc@gmail.com> >> wrote: >> > On Wed, Jan 11, 2017 at 12:58 PM, Martin von Zweigbergk via >> Mercurial-devel >> > <mercurial-devel@mercurial-scm.org> wrote: >> >> >> >> # HG changeset patch >> >> # User Martin von Zweigbergk <martinvonz@google.com> >> >> # Date 1484168228 28800 >> >> # Wed Jan 11 12:57:08 2017 -0800 >> >> # Node ID 193e2d6cba6f294dca88b939a2a1afa5874a9794 >> >> # Parent 9823e2f50a935f6170e01235b65b5282680ebdab >> >> tests: fix import order in test-bdiff >> >> >> >> Without this, I see the following failure in >> >> test-check-module-imports.t. >> >> >> >> @@ -180,3 +180,5 @@ >> >> > -X tests/test-hgweb-no-request-uri.t \ >> >> > -X tests/test-hgweb-non-interactive.t \ >> >> > | sed 's-\\-/-g' | python "$import_checker" - >> >> + tests/test-bdiff.py:6: imports not lexically sorted: >> silenttestrunner < >> >> unittest >> >> + [1] >> >> >> >> diff -r 9823e2f50a93 -r 193e2d6cba6f tests/test-bdiff.py >> >> --- a/tests/test-bdiff.py Sun Jan 08 00:52:54 2017 +0800 >> >> +++ b/tests/test-bdiff.py Wed Jan 11 12:57:08 2017 -0800 >> >> @@ -1,10 +1,9 @@ >> >> from __future__ import absolute_import, print_function >> >> import collections >> >> +import silenttestrunner >> >> import struct >> >> import unittest >> >> >> >> -import silenttestrunner >> >> - >> >> from mercurial import ( >> >> bdiff, >> >> mpatch, >> > >> > >> > Hmmm. This is seemingly a bug in the import checker because >> silenttestrunner >> > is not part of the Python standard library. >> >> So test-verify-repo-operations.py, test-manifest.py and test-lock.py >> are also incorrect in that case? >> > > They look incorrect to me w.r.t. silenttestrunner being in the stdlib > block. test-verify-repo-operations.py is also not even close to using our > import convention, which is why it is excluded by > test-check-module-imports.t. > > >> >> > >> > But, uh, I can't reproduce this failure. If I had to take a guess, it >> would >> > be that you have a silenttestrunner module installed in your Python >> install >> > and that is confusing the import checker into believing it is part of >> the >> > stdlib. Does `import silenttestrunner` work from a Python REPL on your >> > machine? >> >> Nope, it fails. >> > > I still can't reproduce your failure. > > If I run the import checker manually, I do get a different failure: > > $ echo tests/test-bdiff.py | python contrib/import-checker.py - > tests/test-bdiff.py:8: direct symbol import bdiff, mpatch from mercurial > > And if I run a command inspired from test-check-module-imports.t: > > $ hg locate 'set:**.py or grep(r"^#!.*?python")' 'tests/**.t' | python > contrib/import-checker.py - > > I don't see a failure for test-bdiff.py. Wat. > > I have a hunch import-checker.py is doing something wonky when processing > multiple files. > I think this may be true, but I think isn't the full explanation. The reason it's not complaining about the direct symbol import when you run the full set through is probably because of the multiple files. If I instead do `echo tests/test-bdiff.py | python "$import_checker" -` in the test, so just the one file instead of the full set, I get both issues. I haven't tracked down what file(s) it locates that hide the bdiff/mpatch issue, but that's somewhat irrelevant I guess. I think I'm now able to explain why Martin and I are seeing this and you are not - we're both working on our Linux workstations, which at Google has the home directories under /usr/local/google/home/<username>. Meanwhile, sys.prefix and sys.exec_prefix are '/usr' on my machine. So, when import_checker uses /usr as the stdlib_prefixes [1], we get anything in our home directories included in those checks. Since this is, afaict, the only imported thing from tests/ by other things in tests [2], I wonder if it wouldn't make sense to just skip silenttestrunner in import-checker by name, or make import-checker recognize everything under $(dirname $TESTDIR) as not-stdlib. [1] https://www.mercurial-scm.org/repo/hg/file/tip/contrib/import-checker.py#l206 [2] $ for f in $(ls *.py -1 | grep -v 'test-.*.py' | sed 's/.py$//'); do grep '^import '$f *.py; done test-bdiff.py:import silenttestrunner test-ctxmanager.py:import silenttestrunner test-lock.py:import silenttestrunner test-manifest.py:import silenttestrunner test-verify-repo-operations.py:import silenttestrunner > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > >
Initially I fixed a lot of such error messages while adding absolute_import to files. I just send a patch which I guess must prevent the test failure at Google. https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-February/092978.html On Mon, Feb 13, 2017 at 3:03 PM, Kyle Lippincott <spectral@pewpew.net> wrote: > > > On Thu, Jan 12, 2017 at 8:59 PM, Gregory Szorc <gregory.szorc@gmail.com> > wrote: > >> On Wed, Jan 11, 2017 at 5:36 PM, Martin von Zweigbergk < >> martinvonz@google.com> wrote: >> >>> On Wed, Jan 11, 2017 at 4:48 PM, Gregory Szorc <gregory.szorc@gmail.com> >>> wrote: >>> > On Wed, Jan 11, 2017 at 12:58 PM, Martin von Zweigbergk via >>> Mercurial-devel >>> > <mercurial-devel@mercurial-scm.org> wrote: >>> >> >>> >> # HG changeset patch >>> >> # User Martin von Zweigbergk <martinvonz@google.com> >>> >> # Date 1484168228 28800 >>> >> # Wed Jan 11 12:57:08 2017 -0800 >>> >> # Node ID 193e2d6cba6f294dca88b939a2a1afa5874a9794 >>> >> # Parent 9823e2f50a935f6170e01235b65b5282680ebdab >>> >> tests: fix import order in test-bdiff >>> >> >>> >> Without this, I see the following failure in >>> >> test-check-module-imports.t. >>> >> >>> >> @@ -180,3 +180,5 @@ >>> >> > -X tests/test-hgweb-no-request-uri.t \ >>> >> > -X tests/test-hgweb-non-interactive.t \ >>> >> > | sed 's-\\-/-g' | python "$import_checker" - >>> >> + tests/test-bdiff.py:6: imports not lexically sorted: >>> silenttestrunner < >>> >> unittest >>> >> + [1] >>> >> >>> >> diff -r 9823e2f50a93 -r 193e2d6cba6f tests/test-bdiff.py >>> >> --- a/tests/test-bdiff.py Sun Jan 08 00:52:54 2017 +0800 >>> >> +++ b/tests/test-bdiff.py Wed Jan 11 12:57:08 2017 -0800 >>> >> @@ -1,10 +1,9 @@ >>> >> from __future__ import absolute_import, print_function >>> >> import collections >>> >> +import silenttestrunner >>> >> import struct >>> >> import unittest >>> >> >>> >> -import silenttestrunner >>> >> - >>> >> from mercurial import ( >>> >> bdiff, >>> >> mpatch, >>> > >>> > >>> > Hmmm. This is seemingly a bug in the import checker because >>> silenttestrunner >>> > is not part of the Python standard library. >>> >>> So test-verify-repo-operations.py, test-manifest.py and test-lock.py >>> are also incorrect in that case? >>> >> >> They look incorrect to me w.r.t. silenttestrunner being in the stdlib >> block. test-verify-repo-operations.py is also not even close to using our >> import convention, which is why it is excluded by >> test-check-module-imports.t. >> >> >>> >>> > >>> > But, uh, I can't reproduce this failure. If I had to take a guess, it >>> would >>> > be that you have a silenttestrunner module installed in your Python >>> install >>> > and that is confusing the import checker into believing it is part of >>> the >>> > stdlib. Does `import silenttestrunner` work from a Python REPL on your >>> > machine? >>> >>> Nope, it fails. >>> >> >> I still can't reproduce your failure. >> >> If I run the import checker manually, I do get a different failure: >> >> $ echo tests/test-bdiff.py | python contrib/import-checker.py - >> tests/test-bdiff.py:8: direct symbol import bdiff, mpatch from mercurial >> >> And if I run a command inspired from test-check-module-imports.t: >> >> $ hg locate 'set:**.py or grep(r"^#!.*?python")' 'tests/**.t' | python >> contrib/import-checker.py - >> >> I don't see a failure for test-bdiff.py. Wat. >> >> I have a hunch import-checker.py is doing something wonky when processing >> multiple files. >> > > I think this may be true, but I think isn't the full explanation. The > reason it's not complaining about the direct symbol import when you run the > full set through is probably because of the multiple files. If I instead > do `echo tests/test-bdiff.py | python "$import_checker" -` in the test, so > just the one file instead of the full set, I get both issues. I haven't > tracked down what file(s) it locates that hide the bdiff/mpatch issue, but > that's somewhat irrelevant I guess. > > I think I'm now able to explain why Martin and I are seeing this and you > are not - we're both working on our Linux workstations, which at Google has > the home directories under /usr/local/google/home/<username>. Meanwhile, > sys.prefix and sys.exec_prefix are '/usr' on my machine. So, when > import_checker uses /usr as the stdlib_prefixes [1], we get anything in our > home directories included in those checks. > > Since this is, afaict, the only imported thing from tests/ by other things > in tests [2], I wonder if it wouldn't make sense to just skip > silenttestrunner in import-checker by name, or make import-checker > recognize everything under $(dirname $TESTDIR) as not-stdlib. > > [1] https://www.mercurial-scm.org/repo/hg/file/tip/contrib/ > import-checker.py#l206 > [2] > $ for f in $(ls *.py -1 | grep -v 'test-.*.py' | sed 's/.py$//'); do grep > '^import '$f *.py; done > test-bdiff.py:import silenttestrunner > test-ctxmanager.py:import silenttestrunner > test-lock.py:import silenttestrunner > test-manifest.py:import silenttestrunner > test-verify-repo-operations.py:import silenttestrunner > > >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@mercurial-scm.org >> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel >> >> > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > >
Patch
diff -r 9823e2f50a93 -r 193e2d6cba6f tests/test-bdiff.py --- a/tests/test-bdiff.py Sun Jan 08 00:52:54 2017 +0800 +++ b/tests/test-bdiff.py Wed Jan 11 12:57:08 2017 -0800 @@ -1,10 +1,9 @@ from __future__ import absolute_import, print_function import collections +import silenttestrunner import struct import unittest -import silenttestrunner - from mercurial import ( bdiff, mpatch,