commit:     6806341926ca8490a09cc2d3726478c68858ec93
Author:     Michał Górny <mgorny <AT> gentoo <DOT> org>
AuthorDate: Sat Jul 12 15:02:40 2025 +0000
Commit:     Arthur Zamarin <arthurzam <AT> gentoo <DOT> org>
CommitDate: Tue Jul 15 18:08:08 2025 +0000
URL:        
https://gitweb.gentoo.org/proj/pkgcore/pkgcheck.git/commit/?id=68063419

python: Add a check for misplaced `EPYTEST_*` variables

Add a check for `EPYTEST_*` variables being declared after calling
`distutils_enable_tests`, so they don't affect it.  This catches only
global scope use, as it is valid to override them in function scope.
Since using `${var:=...}` assignment is also valid, hack a quick method
to handle them as well.

Closes: https://github.com/pkgcore/pkgcheck/issues/739
Signed-off-by: Michał Górny <mgorny <AT> gentoo.org>
Part-of: https://github.com/pkgcore/pkgcheck/pull/747
Signed-off-by: Arthur Zamarin <arthurzam <AT> gentoo.org>

 src/pkgcheck/bash/__init__.py                      |  1 +
 src/pkgcheck/checks/python.py                      | 72 +++++++++++++++++++++-
 .../PythonCheck/MisplacedEPyTestVar/expected.json  |  4 ++
 .../PythonCheck/MisplacedEPyTestVar/fix.patch      | 19 ++++++
 .../MisplacedEPyTestVar-0.ebuild                   | 31 ++++++++++
 testdata/repos/python/eclass/distutils-r1.eclass   |  2 +
 6 files changed, 127 insertions(+), 2 deletions(-)

diff --git a/src/pkgcheck/bash/__init__.py b/src/pkgcheck/bash/__init__.py
index 1b47881c..8cbacea0 100644
--- a/src/pkgcheck/bash/__init__.py
+++ b/src/pkgcheck/bash/__init__.py
@@ -13,6 +13,7 @@ parser = Parser(language=lang)
 cmd_query = query("(command) @call")
 func_query = query("(function_definition) @func")
 var_assign_query = query("(variable_assignment) @assign")
+var_expansion_query = query("(expansion) @exp")
 var_query = query("(variable_name) @var")
 
 

diff --git a/src/pkgcheck/checks/python.py b/src/pkgcheck/checks/python.py
index bc5c71bb..8249b3b8 100644
--- a/src/pkgcheck/checks/python.py
+++ b/src/pkgcheck/checks/python.py
@@ -247,6 +247,25 @@ class PythonMissingSCMDependency(results.VersionResult, 
results.Warning):
     )
 
 
+class MisplacedEPyTestVar(results.LineResult, results.Error):
+    """Invalid placement of ``EPYTEST_*`` variable
+
+    ``EPYTEST_*`` variables need to be set prior to ``distutils_enable_tests``
+    to enable its functionality.  The exception to that rule are local 
overrides
+    in ``python_test()`` -- we presume the author knows what they're doing.
+    """
+
+    def __init__(self, variable, **kwargs):
+        super().__init__(**kwargs)
+        self.variable = variable
+
+    @property
+    def desc(self):
+        return (
+            f"line {self.lineno}: variable set after calling 
distutils_enable_tests: {self.line!r}"
+        )
+
+
 class PythonCheck(Check):
     """Python eclass checks.
 
@@ -256,7 +275,7 @@ class PythonCheck(Check):
 
     _source = sources.EbuildParseRepoSource
     known_results = frozenset(
-        [
+        {
             MissingPythonEclass,
             PythonMissingRequiredUse,
             PythonMissingDeps,
@@ -268,7 +287,8 @@ class PythonCheck(Check):
             PythonAnyMismatchedUseHasVersionCheck,
             PythonAnyMismatchedDepHasVersionCheck,
             PythonMissingSCMDependency,
-        ]
+            MisplacedEPyTestVar,
+        }
     )
 
     has_version_known_flags = {
@@ -389,6 +409,53 @@ class PythonCheck(Check):
             if not self.setuptools_scm.intersection(bdepends):
                 yield PythonMissingSCMDependency(pkg=pkg)
 
+    def _get_all_global_assignments(self, pkg):
+        """Iterate over global plain and expansion assignments"""
+        for node in pkg.global_query(bash.var_assign_query):
+            name = pkg.node_str(node.child_by_field_name("name"))
+            yield (name, node)
+
+        # ${var:=value} expansions.
+        for node in pkg.global_query(bash.var_expansion_query):
+            if node.children[0].text != b"${":
+                continue
+            name = node.children[1].text
+            if node.children[2].text not in (b"=", b":="):
+                continue
+            yield (name.decode(), node)
+
+    def check_epytest_vars(self, pkg):
+        """Check for incorrect use of EPYTEST_* variables"""
+        # TODO: do we want to check for multiple det calls? Quite unlikely.
+        for node in pkg.global_query(bash.cmd_query):
+            name = pkg.node_str(node.child_by_field_name("name"))
+            if name != "distutils_enable_tests":
+                continue
+            for argument_node in node.children_by_field_name("argument"):
+                argument = pkg.node_str(argument_node)
+                # Check if the first argument is "pytest", but we need
+                # to skip line continuations explicitly.
+                if argument == "pytest":
+                    det_lineno, _ = node.start_point
+                    break
+                elif argument != "\\":
+                    return
+            else:
+                return
+
+        for var_name, var_node in self._get_all_global_assignments(pkg):
+            # While not all variables affect distutils_enable_tests, make it
+            # future proof.  Also, it makes sense to keep them together.
+            # Just opt out from the long lists.
+            if var_name.startswith("EPYTEST_") and var_name not in (
+                "EPYTEST_DESELECT",
+                "EPYTEST_IGNORE",
+            ):
+                lineno, _ = var_node.start_point
+                if lineno > det_lineno:
+                    line = pkg.node_str(var_node)
+                    yield MisplacedEPyTestVar(var_name, line=line, 
lineno=lineno + 1, pkg=pkg)
+
     @staticmethod
     def _prepare_deps(deps: str):
         try:
@@ -545,6 +612,7 @@ class PythonCheck(Check):
 
         if "distutils-r1" in pkg.inherited:
             yield from self.check_pep517(pkg)
+            yield from self.check_epytest_vars(pkg)
 
         any_dep_func = self.eclass_any_dep_func[eclass]
         python_check_deps = self.build_python_gen_any_dep_calls(pkg, 
any_dep_func)

diff --git 
a/testdata/data/repos/python/PythonCheck/MisplacedEPyTestVar/expected.json 
b/testdata/data/repos/python/PythonCheck/MisplacedEPyTestVar/expected.json
new file mode 100644
index 00000000..8af51362
--- /dev/null
+++ b/testdata/data/repos/python/PythonCheck/MisplacedEPyTestVar/expected.json
@@ -0,0 +1,4 @@
+{"__class__": "MisplacedEPyTestVar", "category": "PythonCheck", "package": 
"MisplacedEPyTestVar", "version": "0", "line": "EPYTEST_PLUGIN_AUTOLOAD=1", 
"lineno": 15, "variable": "EPYTEST_PLUGIN_AUTOLOAD"}
+{"__class__": "MisplacedEPyTestVar", "category": "PythonCheck", "package": 
"MisplacedEPyTestVar", "version": "0", "line": "EPYTEST_PLUGINS=( foo bar baz 
)", "lineno": 16, "variable": "EPYTEST_PLUGINS"}
+{"__class__": "MisplacedEPyTestVar", "category": "PythonCheck", "package": 
"MisplacedEPyTestVar", "version": "0", "line": "EPYTEST_XDIST=1", "lineno": 17, 
"variable": "EPYTEST_XDIST"}
+{"__class__": "MisplacedEPyTestVar", "category": "PythonCheck", "package": 
"MisplacedEPyTestVar", "version": "0", "line": "${EPYTEST_TIMEOUT:=180}", 
"lineno": 18, "variable": "EPYTEST_TIMEOUT"}

diff --git 
a/testdata/data/repos/python/PythonCheck/MisplacedEPyTestVar/fix.patch 
b/testdata/data/repos/python/PythonCheck/MisplacedEPyTestVar/fix.patch
new file mode 100644
index 00000000..7329ebb9
--- /dev/null
+++ b/testdata/data/repos/python/PythonCheck/MisplacedEPyTestVar/fix.patch
@@ -0,0 +1,19 @@
+diff '--color=auto' -Naur 
python/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild 
fixed/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild
+--- python/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild        
2025-07-12 17:10:51.665298954 +0200
++++ fixed/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild 
2025-07-12 17:15:30.258231253 +0200
+@@ -10,13 +10,13 @@
+ LICENSE="BSD"
+ SLOT="0"
+ 
+-distutils_enable_tests pytest
+-
+ EPYTEST_PLUGIN_AUTOLOAD=1
+ EPYTEST_PLUGINS=( foo bar baz )
+ EPYTEST_XDIST=1
+ : ${EPYTEST_TIMEOUT:=180}
+ 
++distutils_enable_tests pytest
++
+ EPYTEST_DESELECT=(
+       tests/test_foo.py::test_foo
+ )

diff --git 
a/testdata/repos/python/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild
 
b/testdata/repos/python/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild
new file mode 100644
index 00000000..d40ba1e4
--- /dev/null
+++ 
b/testdata/repos/python/PythonCheck/MisplacedEPyTestVar/MisplacedEPyTestVar-0.ebuild
@@ -0,0 +1,31 @@
+EAPI=8
+
+DISTUTILS_USE_PEP517=flit
+PYTHON_COMPAT=( python3_10 )
+
+inherit distutils-r1
+
+DESCRIPTION="Ebuild with misplaced EPYTEST vars"
+HOMEPAGE="https://github.com/pkgcore/pkgcheck";
+LICENSE="BSD"
+SLOT="0"
+
+distutils_enable_tests pytest
+
+EPYTEST_PLUGIN_AUTOLOAD=1
+EPYTEST_PLUGINS=( foo bar baz )
+EPYTEST_XDIST=1
+: ${EPYTEST_TIMEOUT:=180}
+
+EPYTEST_DESELECT=(
+       tests/test_foo.py::test_foo
+)
+EPYTEST_IGNORE=(
+       tests/test_bar.py
+)
+
+python_test() {
+       : ${EPYTEST_TIMEOUT:=300}
+       local EPYTEST_PLUGINS=( "${EPYTEST_PLUGINS[@]}" more )
+       EPYTEST_XDIST= epytest
+}

diff --git a/testdata/repos/python/eclass/distutils-r1.eclass 
b/testdata/repos/python/eclass/distutils-r1.eclass
index 37d69cd0..6e6d9d06 100644
--- a/testdata/repos/python/eclass/distutils-r1.eclass
+++ b/testdata/repos/python/eclass/distutils-r1.eclass
@@ -58,3 +58,5 @@ _distutils_set_globals() {
 }
 _distutils_set_globals
 unset -f _distutils_set_globals
+
+distutils_enable_tests() { :; }

Reply via email to