Re: [PATCH i-g-t 1/1] scripts/test_list: relax treatment of non-compiled tests

["Manszewski, Christoph" <christoph.manszewski@intel.com>]

Hi Kamil,

On 12.09.2024 16:04, Kamil Konieczny wrote:
> Some tests could be added and compiled only when non-default
> meson option is given. It will result in no testlist generated
> for such tests but they could appear in our documentation.
> Currently we report an error in such case.
> 
> Create a way to detect and report that and treat this as feature
> and do not report it as an error.  This should allow to relax
> processing of chamelium and other new tests.
> 
> Also while at this, print error information before actually
> exiting with an error code, as previous prints were only
> warnings.
> 
> Cc: Christoph Manszewski <christoph.manszewski@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Jari Tahvanainen <jari.tahvanainen@intel.com>
> Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
>   scripts/test_list.py | 31 +++++++++++++++++++++++++++++--
>   1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/test_list.py b/scripts/test_list.py
> index 69c830ca1..322933c26 100644
> --- a/scripts/test_list.py
> +++ b/scripts/test_list.py
> @@ -1141,6 +1141,20 @@ class TestList:
>   
>           return sorted(tests)
>   
> +    def get_notcompiled(self):
> +
> +        """ Return a list of tests which were not compiled """
> +        no_binaries = []
> +        for name in self.filenames:
> +            test_basename = re.sub(r"\.c$", "", name.split('/')[-1])
> +            fname = os.path.join(self.igt_build_path, "tests", test_basename)
> +
> +            if not os.path.isfile(fname):
> +                print(f"{test_basename}: No binary found, test was not compiled.")
> +                no_binaries.append(test_basename)
> +
> +        return sorted(no_binaries)
> +
>       #
>       # Validation methods
>       #
> @@ -1177,21 +1191,34 @@ class TestList:
>           # Get a list of tests from
>           run_subtests = set(self.get_testlist())
>   
> +        # Get a list of not compiled tests
> +        no_binary = set(self.get_notcompiled())
> +
>           # Compare sets
>   
>           run_missing = list(sorted(run_subtests - doc_subtests))
>           doc_uneeded = list(sorted(doc_subtests - run_subtests))
>   
> +        """Check if there are (possibly unneeded) documented subtests"""
>           if doc_uneeded:
> +            un_reported = []
>               for test_name in doc_uneeded:
> -                print(f"Warning: Documented {test_name} doesn't exist on source files")
> +                test_basename = test_name.split('@')[1]
> +                if test_basename in no_binary:
> +                    if test_basename not in un_reported:
> +                        print(f"Info: Documented {test_name} don't have a binary {test_basename}")
> +                        un_reported.append(test_basename)
> +                else:
> +                    print(f"Info: Documented {test_name} doesn't exist on source files")
>   
> +        """Check for missing documentation"""
>           if run_missing:
>               for test_name in run_missing:
>                   print(f'Warning: Missing documentation for {test_name}')
>               print('Please refer: docs/test_documentation.md for more details')
>   
> -        if doc_uneeded or run_missing:
> +        if run_missing:
> +            print('Error: Missing tests documentation')
>               sys.exit(1)

I would suggest to relax this check a little bit less, i.e. allow for 
documentation of tests that were not compiled, but disallow 
documentation for sub tests that are missing in compiled files. This 
leaves the benefit of ensuring there is no leftover documentation.

Something like:

+    def get_not_compiled(self):
+
+        """ Return a list of tests which were not compiled """
+        no_binaries = []
+        for name in self.filenames:
+            test_basename = re.sub(r"\.c$", "", name.split('/')[-1])
+            fname = os.path.join(self.igt_build_path, "tests", 
test_basename)
+
+            if not os.path.isfile(fname):
+                no_binaries.append(test_basename)
+
+        return sorted(no_binaries)
+
      #
      # Validation methods
      #
@@ -1177,21 +1190,24 @@ class TestList:
          # Get a list of tests from
          run_subtests = set(self.get_testlist())

+        not_compiled = set(self.get_not_compiled())
+        for test_basename in not_compiled:
+            print(f"INFO: Found documentation for '{test_basename}' but 
no binary")
+
          # Compare sets

          run_missing = list(sorted(run_subtests - doc_subtests))
          doc_uneeded = list(sorted(doc_subtests - run_subtests))
+        doc_uneeded_build = [t for t in doc_uneeded if t.split('@')[1] 
not in not_compiled]

-        if doc_uneeded:
-            for test_name in doc_uneeded:
-                print(f"Warning: Documented {test_name} doesn't exist 
on source files")
-
-        if run_missing:
+        if doc_uneeded_build or run_missing:
              for test_name in run_missing:
-                print(f'Warning: Missing documentation for {test_name}')
-            print('Please refer: docs/test_documentation.md for more 
details')
+                print(f'ERROR: Missing documentation for {test_name}')

-        if doc_uneeded or run_missing:
+            for test_name in doc_uneeded_build:
+                print(f'ERROR: Unneeded documentation for {test_name}')
+
+            print('Please refer: docs/test_documentation.md for more 
details')
              sys.exit(1)

Anyway, as discussed offline I will grab your change and integrate it 
into the eudebug series.

Thanks,
Christoph


>   
>       #