Patchwork import-checkers: split tests of the tool from running it on the source

login
register
mail settings
Submitter Pierre-Yves David
Date March 15, 2017, 6:16 a.m.
Message ID <6e53450f1d41962edb5d.1489558596@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/19346/
State Accepted
Headers show

Comments

Pierre-Yves David - March 15, 2017, 6:16 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1489558028 25200
#      Tue Mar 14 23:07:08 2017 -0700
# Node ID 6e53450f1d41962edb5df6c2ff35d79adc04c775
# Parent  ed23f929af38e3249a4c0b258939e89782b20795
# EXP-Topic import-checkers
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 6e53450f1d41
import-checkers: split tests of the tool from running it on the source

We did such splits for other tools already. The 'test-check-*.t' performs the
check of the source code while the regular tests verifies the tools works.

One of the benefit is that is provides a simple file to reuse in third party
extensions.
Augie Fackler - March 16, 2017, 5:02 p.m.
On Tue, Mar 14, 2017 at 11:16:36PM -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1489558028 25200
> #      Tue Mar 14 23:07:08 2017 -0700
> # Node ID 6e53450f1d41962edb5df6c2ff35d79adc04c775
> # Parent  ed23f929af38e3249a4c0b258939e89782b20795
> # EXP-Topic import-checkers
> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 6e53450f1d41
> import-checkers: split tests of the tool from running it on the source

Sure, queued, thanks.

>
> We did such splits for other tools already. The 'test-check-*.t' performs the
> check of the source code while the regular tests verifies the tools works.
>
> One of the benefit is that is provides a simple file to reuse in third party
> extensions.
>
> diff --git a/tests/test-check-module-imports.t b/tests/test-check-module-imports.t
> --- a/tests/test-check-module-imports.t
> +++ b/tests/test-check-module-imports.t
> @@ -3,148 +3,6 @@
>    $ . "$TESTDIR/helpers-testrepo.sh"
>    $ import_checker="$TESTDIR"/../contrib/import-checker.py
>
> -Run the doctests from the import checker, and make sure
> -it's working correctly.
> -  $ TERM=dumb
> -  $ export TERM
> -  $ python -m doctest $import_checker
> -
> -Run additional tests for the import checker
> -
> -  $ mkdir testpackage
> -  $ touch testpackage/__init__.py
> -
> -  $ cat > testpackage/multiple.py << EOF
> -  > from __future__ import absolute_import
> -  > import os, sys
> -  > EOF
> -
> -  $ cat > testpackage/unsorted.py << EOF
> -  > from __future__ import absolute_import
> -  > import sys
> -  > import os
> -  > EOF
> -
> -  $ cat > testpackage/stdafterlocal.py << EOF
> -  > from __future__ import absolute_import
> -  > from . import unsorted
> -  > import os
> -  > EOF
> -
> -  $ cat > testpackage/requirerelative.py << EOF
> -  > from __future__ import absolute_import
> -  > import testpackage.unsorted
> -  > EOF
> -
> -  $ cat > testpackage/importalias.py << EOF
> -  > from __future__ import absolute_import
> -  > import ui
> -  > EOF
> -
> -  $ cat > testpackage/relativestdlib.py << EOF
> -  > from __future__ import absolute_import
> -  > from .. import os
> -  > EOF
> -
> -  $ cat > testpackage/symbolimport.py << EOF
> -  > from __future__ import absolute_import
> -  > from .unsorted import foo
> -  > EOF
> -
> -  $ cat > testpackage/latesymbolimport.py << EOF
> -  > from __future__ import absolute_import
> -  > from . import unsorted
> -  > from mercurial.node import hex
> -  > EOF
> -
> -  $ cat > testpackage/multiplegroups.py << EOF
> -  > from __future__ import absolute_import
> -  > from . import unsorted
> -  > from . import more
> -  > EOF
> -
> -  $ mkdir testpackage/subpackage
> -  $ cat > testpackage/subpackage/levelpriority.py << EOF
> -  > from __future__ import absolute_import
> -  > from . import foo
> -  > from .. import parent
> -  > EOF
> -
> -  $ touch testpackage/subpackage/foo.py
> -  $ cat > testpackage/subpackage/__init__.py << EOF
> -  > from __future__ import absolute_import
> -  > from . import levelpriority  # should not cause cycle
> -  > EOF
> -
> -  $ cat > testpackage/subpackage/localimport.py << EOF
> -  > from __future__ import absolute_import
> -  > from . import foo
> -  > def bar():
> -  >     # should not cause "higher-level import should come first"
> -  >     from .. import unsorted
> -  >     # but other errors should be detected
> -  >     from .. import more
> -  >     import testpackage.subpackage.levelpriority
> -  > EOF
> -
> -  $ cat > testpackage/importmodulefromsub.py << EOF
> -  > from __future__ import absolute_import
> -  > from .subpackage import foo  # not a "direct symbol import"
> -  > EOF
> -
> -  $ cat > testpackage/importsymbolfromsub.py << EOF
> -  > from __future__ import absolute_import
> -  > from .subpackage import foo, nonmodule
> -  > EOF
> -
> -  $ cat > testpackage/sortedentries.py << EOF
> -  > from __future__ import absolute_import
> -  > from . import (
> -  >     foo,
> -  >     bar,
> -  > )
> -  > EOF
> -
> -  $ cat > testpackage/importfromalias.py << EOF
> -  > from __future__ import absolute_import
> -  > from . import ui
> -  > EOF
> -
> -  $ cat > testpackage/importfromrelative.py << EOF
> -  > from __future__ import absolute_import
> -  > from testpackage.unsorted import foo
> -  > EOF
> -
> -  $ mkdir testpackage2
> -  $ touch testpackage2/__init__.py
> -
> -  $ cat > testpackage2/latesymbolimport.py << EOF
> -  > from __future__ import absolute_import
> -  > from testpackage import unsorted
> -  > from mercurial.node import hex
> -  > EOF
> -
> -  $ python "$import_checker" testpackage*/*.py testpackage/subpackage/*.py
> -  testpackage/importalias.py:2: ui module must be "as" aliased to uimod
> -  testpackage/importfromalias.py:2: ui from testpackage must be "as" aliased to uimod
> -  testpackage/importfromrelative.py:2: import should be relative: testpackage.unsorted
> -  testpackage/importfromrelative.py:2: direct symbol import foo from testpackage.unsorted
> -  testpackage/importsymbolfromsub.py:2: direct symbol import nonmodule from testpackage.subpackage
> -  testpackage/latesymbolimport.py:3: symbol import follows non-symbol import: mercurial.node
> -  testpackage/multiple.py:2: multiple imported names: os, sys
> -  testpackage/multiplegroups.py:3: multiple "from . import" statements
> -  testpackage/relativestdlib.py:2: relative import of stdlib module
> -  testpackage/requirerelative.py:2: import should be relative: testpackage.unsorted
> -  testpackage/sortedentries.py:2: imports from testpackage not lexically sorted: bar < foo
> -  testpackage/stdafterlocal.py:3: stdlib import "os" follows local import: testpackage
> -  testpackage/subpackage/levelpriority.py:3: higher-level import should come first: testpackage
> -  testpackage/subpackage/localimport.py:7: multiple "from .. import" statements
> -  testpackage/subpackage/localimport.py:8: import should be relative: testpackage.subpackage.levelpriority
> -  testpackage/symbolimport.py:2: direct symbol import foo from testpackage.unsorted
> -  testpackage/unsorted.py:3: imports not lexically sorted: os < sys
> -  testpackage2/latesymbolimport.py:3: symbol import follows non-symbol import: mercurial.node
> -  [1]
> -
>    $ cd "$TESTDIR"/..
>
>  There are a handful of cases here that require renaming a module so it
> @@ -171,7 +29,7 @@ outputs, which should be fixed later.
>    > -X tests/test-verify-repo-operations.py \
>    > -X tests/test-hook.t \
>    > -X tests/test-import.t \
> -  > -X tests/test-check-module-imports.t \
> +  > -X tests/test-imports-checker.t \
>    > -X tests/test-commit-interactive.t \
>    > -X tests/test-contrib-check-code.t \
>    > -X tests/test-extension.t \
> diff --git a/tests/test-check-module-imports.t b/tests/test-imports-checker.t
> copy from tests/test-check-module-imports.t
> copy to tests/test-imports-checker.t
> --- a/tests/test-check-module-imports.t
> +++ b/tests/test-imports-checker.t
> @@ -144,39 +144,3 @@ Run additional tests for the import chec
>    testpackage/unsorted.py:3: imports not lexically sorted: os < sys
>    testpackage2/latesymbolimport.py:3: symbol import follows non-symbol import: mercurial.node
>    [1]
> -
> -  $ cd "$TESTDIR"/..
> -
> -There are a handful of cases here that require renaming a module so it
> -doesn't overlap with a stdlib module name. There are also some cycles
> -here that we should still endeavor to fix, and some cycles will be
> -hidden by deduplication algorithm in the cycle detector, so fixing
> -these may expose other cycles.
> -
> -Known-bad files are excluded by -X as some of them would produce unstable
> -outputs, which should be fixed later.
> -
> -  $ hg locate 'set:**.py or grep(r"^#!.*?python")' \
> -  > 'tests/**.t' \
> -  > -X contrib/debugshell.py \
> -  > -X contrib/python-zstandard/ \
> -  > -X contrib/win32/hgwebdir_wsgi.py \
> -  > -X doc/gendoc.py \
> -  > -X doc/hgmanpage.py \
> -  > -X i18n/posplit \
> -  > -X tests/test-hgweb-auth.py \
> -  > -X tests/hypothesishelpers.py \
> -  > -X tests/test-ctxmanager.py \
> -  > -X tests/test-lock.py \
> -  > -X tests/test-verify-repo-operations.py \
> -  > -X tests/test-hook.t \
> -  > -X tests/test-import.t \
> -  > -X tests/test-check-module-imports.t \
> -  > -X tests/test-commit-interactive.t \
> -  > -X tests/test-contrib-check-code.t \
> -  > -X tests/test-extension.t \
> -  > -X tests/test-hghave.t \
> -  > -X tests/test-hgweb-no-path-info.t \
> -  > -X tests/test-hgweb-no-request-uri.t \
> -  > -X tests/test-hgweb-non-interactive.t \
> -  > | sed 's-\\-/-g' | python "$import_checker" -
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/tests/test-check-module-imports.t b/tests/test-check-module-imports.t
--- a/tests/test-check-module-imports.t
+++ b/tests/test-check-module-imports.t
@@ -3,148 +3,6 @@ 
   $ . "$TESTDIR/helpers-testrepo.sh"
   $ import_checker="$TESTDIR"/../contrib/import-checker.py
 
-Run the doctests from the import checker, and make sure
-it's working correctly.
-  $ TERM=dumb
-  $ export TERM
-  $ python -m doctest $import_checker
-
-Run additional tests for the import checker
-
-  $ mkdir testpackage
-  $ touch testpackage/__init__.py
-
-  $ cat > testpackage/multiple.py << EOF
-  > from __future__ import absolute_import
-  > import os, sys
-  > EOF
-
-  $ cat > testpackage/unsorted.py << EOF
-  > from __future__ import absolute_import
-  > import sys
-  > import os
-  > EOF
-
-  $ cat > testpackage/stdafterlocal.py << EOF
-  > from __future__ import absolute_import
-  > from . import unsorted
-  > import os
-  > EOF
-
-  $ cat > testpackage/requirerelative.py << EOF
-  > from __future__ import absolute_import
-  > import testpackage.unsorted
-  > EOF
-
-  $ cat > testpackage/importalias.py << EOF
-  > from __future__ import absolute_import
-  > import ui
-  > EOF
-
-  $ cat > testpackage/relativestdlib.py << EOF
-  > from __future__ import absolute_import
-  > from .. import os
-  > EOF
-
-  $ cat > testpackage/symbolimport.py << EOF
-  > from __future__ import absolute_import
-  > from .unsorted import foo
-  > EOF
-
-  $ cat > testpackage/latesymbolimport.py << EOF
-  > from __future__ import absolute_import
-  > from . import unsorted
-  > from mercurial.node import hex
-  > EOF
-
-  $ cat > testpackage/multiplegroups.py << EOF
-  > from __future__ import absolute_import
-  > from . import unsorted
-  > from . import more
-  > EOF
-
-  $ mkdir testpackage/subpackage
-  $ cat > testpackage/subpackage/levelpriority.py << EOF
-  > from __future__ import absolute_import
-  > from . import foo
-  > from .. import parent
-  > EOF
-
-  $ touch testpackage/subpackage/foo.py
-  $ cat > testpackage/subpackage/__init__.py << EOF
-  > from __future__ import absolute_import
-  > from . import levelpriority  # should not cause cycle
-  > EOF
-
-  $ cat > testpackage/subpackage/localimport.py << EOF
-  > from __future__ import absolute_import
-  > from . import foo
-  > def bar():
-  >     # should not cause "higher-level import should come first"
-  >     from .. import unsorted
-  >     # but other errors should be detected
-  >     from .. import more
-  >     import testpackage.subpackage.levelpriority
-  > EOF
-
-  $ cat > testpackage/importmodulefromsub.py << EOF
-  > from __future__ import absolute_import
-  > from .subpackage import foo  # not a "direct symbol import"
-  > EOF
-
-  $ cat > testpackage/importsymbolfromsub.py << EOF
-  > from __future__ import absolute_import
-  > from .subpackage import foo, nonmodule
-  > EOF
-
-  $ cat > testpackage/sortedentries.py << EOF
-  > from __future__ import absolute_import
-  > from . import (
-  >     foo,
-  >     bar,
-  > )
-  > EOF
-
-  $ cat > testpackage/importfromalias.py << EOF
-  > from __future__ import absolute_import
-  > from . import ui
-  > EOF
-
-  $ cat > testpackage/importfromrelative.py << EOF
-  > from __future__ import absolute_import
-  > from testpackage.unsorted import foo
-  > EOF
-
-  $ mkdir testpackage2
-  $ touch testpackage2/__init__.py
-
-  $ cat > testpackage2/latesymbolimport.py << EOF
-  > from __future__ import absolute_import
-  > from testpackage import unsorted
-  > from mercurial.node import hex
-  > EOF
-
-  $ python "$import_checker" testpackage*/*.py testpackage/subpackage/*.py
-  testpackage/importalias.py:2: ui module must be "as" aliased to uimod
-  testpackage/importfromalias.py:2: ui from testpackage must be "as" aliased to uimod
-  testpackage/importfromrelative.py:2: import should be relative: testpackage.unsorted
-  testpackage/importfromrelative.py:2: direct symbol import foo from testpackage.unsorted
-  testpackage/importsymbolfromsub.py:2: direct symbol import nonmodule from testpackage.subpackage
-  testpackage/latesymbolimport.py:3: symbol import follows non-symbol import: mercurial.node
-  testpackage/multiple.py:2: multiple imported names: os, sys
-  testpackage/multiplegroups.py:3: multiple "from . import" statements
-  testpackage/relativestdlib.py:2: relative import of stdlib module
-  testpackage/requirerelative.py:2: import should be relative: testpackage.unsorted
-  testpackage/sortedentries.py:2: imports from testpackage not lexically sorted: bar < foo
-  testpackage/stdafterlocal.py:3: stdlib import "os" follows local import: testpackage
-  testpackage/subpackage/levelpriority.py:3: higher-level import should come first: testpackage
-  testpackage/subpackage/localimport.py:7: multiple "from .. import" statements
-  testpackage/subpackage/localimport.py:8: import should be relative: testpackage.subpackage.levelpriority
-  testpackage/symbolimport.py:2: direct symbol import foo from testpackage.unsorted
-  testpackage/unsorted.py:3: imports not lexically sorted: os < sys
-  testpackage2/latesymbolimport.py:3: symbol import follows non-symbol import: mercurial.node
-  [1]
-
   $ cd "$TESTDIR"/..
 
 There are a handful of cases here that require renaming a module so it
@@ -171,7 +29,7 @@  outputs, which should be fixed later.
   > -X tests/test-verify-repo-operations.py \
   > -X tests/test-hook.t \
   > -X tests/test-import.t \
-  > -X tests/test-check-module-imports.t \
+  > -X tests/test-imports-checker.t \
   > -X tests/test-commit-interactive.t \
   > -X tests/test-contrib-check-code.t \
   > -X tests/test-extension.t \
diff --git a/tests/test-check-module-imports.t b/tests/test-imports-checker.t
copy from tests/test-check-module-imports.t
copy to tests/test-imports-checker.t
--- a/tests/test-check-module-imports.t
+++ b/tests/test-imports-checker.t
@@ -144,39 +144,3 @@  Run additional tests for the import chec
   testpackage/unsorted.py:3: imports not lexically sorted: os < sys
   testpackage2/latesymbolimport.py:3: symbol import follows non-symbol import: mercurial.node
   [1]
-
-  $ cd "$TESTDIR"/..
-
-There are a handful of cases here that require renaming a module so it
-doesn't overlap with a stdlib module name. There are also some cycles
-here that we should still endeavor to fix, and some cycles will be
-hidden by deduplication algorithm in the cycle detector, so fixing
-these may expose other cycles.
-
-Known-bad files are excluded by -X as some of them would produce unstable
-outputs, which should be fixed later.
-
-  $ hg locate 'set:**.py or grep(r"^#!.*?python")' \
-  > 'tests/**.t' \
-  > -X contrib/debugshell.py \
-  > -X contrib/python-zstandard/ \
-  > -X contrib/win32/hgwebdir_wsgi.py \
-  > -X doc/gendoc.py \
-  > -X doc/hgmanpage.py \
-  > -X i18n/posplit \
-  > -X tests/test-hgweb-auth.py \
-  > -X tests/hypothesishelpers.py \
-  > -X tests/test-ctxmanager.py \
-  > -X tests/test-lock.py \
-  > -X tests/test-verify-repo-operations.py \
-  > -X tests/test-hook.t \
-  > -X tests/test-import.t \
-  > -X tests/test-check-module-imports.t \
-  > -X tests/test-commit-interactive.t \
-  > -X tests/test-contrib-check-code.t \
-  > -X tests/test-extension.t \
-  > -X tests/test-hghave.t \
-  > -X tests/test-hgweb-no-path-info.t \
-  > -X tests/test-hgweb-no-request-uri.t \
-  > -X tests/test-hgweb-non-interactive.t \
-  > | sed 's-\\-/-g' | python "$import_checker" -