[PATCH] tests: fix import order in test-bdiff

Pulkit Goyal 7895pulkit at gmail.com
Mon Feb 13 15:34:55 EST 2017


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 at pewpew.net>
wrote:

>
>
> On Thu, Jan 12, 2017 at 8:59 PM, Gregory Szorc <gregory.szorc at gmail.com>
> wrote:
>
>> On Wed, Jan 11, 2017 at 5:36 PM, Martin von Zweigbergk <
>> martinvonz at google.com> wrote:
>>
>>> On Wed, Jan 11, 2017 at 4:48 PM, Gregory Szorc <gregory.szorc at gmail.com>
>>> wrote:
>>> > On Wed, Jan 11, 2017 at 12:58 PM, Martin von Zweigbergk via
>>> Mercurial-devel
>>> > <mercurial-devel at mercurial-scm.org> wrote:
>>> >>
>>> >> # HG changeset patch
>>> >> # User Martin von Zweigbergk <martinvonz at 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 at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>
>>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170214/cd55db4c/attachment.html>


More information about the Mercurial-devel mailing list