On Thu, Jan 12, 2017 at 8:59 PM, Gregory Szorc <gregory.sz...@gmail.com> wrote:
> On Wed, Jan 11, 2017 at 5:36 PM, Martin von Zweigbergk < > martinv...@google.com> wrote: > >> On Wed, Jan 11, 2017 at 4:48 PM, Gregory Szorc <gregory.sz...@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 <martinv...@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
