[PATCH i-g-t v2 0/8] Fix several issues when producing Intel CI testlists

[PATCH i-g-t v2 0/8] Fix several issues when producing Intel CI testlists

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Handling test lists and block lists is not trivial: it requires a
complex logic to handle all nuances.

This series fix a series of bugs that were making such lists
unreliable.

While here, also fix a small typo at scripts/test_list.py.

---

v2: rebase on the top of current upstream

Mauro Carvalho Chehab (8):
  scripts/test_list.py: fix a typo on default-if-not-excluded
  scripts/igt_doc.py: better handle issues with empty testlists
  scripts/igt_doc.py: handle empty run_type
  scripts/igt_doc.py: only create dict if subtest will be added
  scripts/igt_doc.py: better handle default gpu
  scripts/igt_doc.py: remove some unused logic
  scripts/igt_doc.py: use an ancillary function to simplify the code
  scripts/igt_doc.py: simplify and cleanup tests_per_list logic

 scripts/igt_doc.py   | 80 ++++++++++++++++++++++++--------------------
 scripts/test_list.py |  2 +-
 2 files changed, 45 insertions(+), 37 deletions(-)

-- 
2.43.0

[PATCH i-g-t v2 1/8] scripts/test_list.py: fix a typo on default-if-not-excluded

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

There is an extra hyphen there.

Currenty, this is harmless, as such config attribut is not
used anymore on current config files.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/test_list.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/test_list.py b/scripts/test_list.py
index eeb452091fbf..69c830ca1539 100644
--- a/scripts/test_list.py
+++ b/scripts/test_list.py
@@ -550,7 +550,7 @@ class TestList:
             if not update:
                 continue
 
-            default_value = update.get("default--if-not-excluded")
+            default_value = update.get("default-if-not-excluded")
             append_value = update.get("append-value-if-not-excluded")
 
             testname = subtest_dict["_summary_"]
-- 
2.43.0

[PATCH i-g-t v2 2/8] scripts/igt_doc.py: better handle issues with empty testlists

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

The warning message of not creating an empty testlist is
harmless, and it should only be used when not all GPUs
are listed. So, change the logic to better check for "other"
testlists before printing a message.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/igt_doc.py | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/scripts/igt_doc.py b/scripts/igt_doc.py
index 6a1cfc2b5a56..a6cbeee067fe 100755
--- a/scripts/igt_doc.py
+++ b/scripts/igt_doc.py
@@ -218,16 +218,14 @@ class IntelciTestlist:
 
                 for testlist, subtests in names.items():
                     if testlist == "":
-                        if not subtests:
-                            continue
-
                         testlist = "other"
                     else:
                         testlist = re.sub(r"[\W_]+", "-", testlist).lower()
                         testlist = re.sub(r"_+", "_", testlist)
 
                     if not subtests:
-                        sys.stderr.write(f"Warning: empty testlist: {testlist}\n")
+                        if testlist != "other":
+                            sys.stderr.write(f"Not creating empty testlist: {dname}/{testlist}\n")
                         continue
 
                     fname = os.path.join(dname, testlist) + ".testlist"
-- 
2.43.0

[PATCH i-g-t v2 3/8] scripts/igt_doc.py: handle empty run_type

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

If run_type is not set, change it to "other".

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/igt_doc.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/scripts/igt_doc.py b/scripts/igt_doc.py
index a6cbeee067fe..8a10db13081c 100755
--- a/scripts/igt_doc.py
+++ b/scripts/igt_doc.py
@@ -85,6 +85,9 @@ class IgtTestList(TestList):
                     tests_per_list[driver] = {}
 
                 for run_type in run_type_set:
+                    if not run_type:
+                        run_type = "other"
+
                     if run_type not in tests_per_list[driver]:
                         tests_per_list[driver][run_type] = {}
 
@@ -108,6 +111,10 @@ class IgtTestList(TestList):
         for driver, run_types in tests_per_list.items():
             testlists[driver] = {}
             for run_type, subnames in run_types.items():
+
+                if not run_type:
+                    run_type = "other"
+
                 for subname, gpus in subnames.items():
                     if not gpu_set:
                         gpu = "default"
-- 
2.43.0

[PATCH i-g-t v2 4/8] scripts/igt_doc.py: only create dict if subtest will be added

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Currently, depending on the order of the sets/dict, some
empty testlists are created. Change the logic to only add
an entry into the dict if a subtest will be added to it.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/igt_doc.py | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/scripts/igt_doc.py b/scripts/igt_doc.py
index 8a10db13081c..1c2c7a8ef973 100755
--- a/scripts/igt_doc.py
+++ b/scripts/igt_doc.py
@@ -116,31 +116,31 @@ class IgtTestList(TestList):
                     run_type = "other"
 
                 for subname, gpus in subnames.items():
-                    if not gpu_set:
-                        gpu = "default"
-
-                    if gpu not in testlists[driver]:
-                        testlists[driver][gpu] = {}
-
-                    if run_type not in testlists[driver][gpu]:
-                        testlists[driver][gpu][run_type] = set()
-
-                    # Trivial case: fields not defined
-                    if not gpu_set:
-                        testlists[driver][gpu][run_type].add(subname)
-                        continue
-
-                    # Globally blocklisted values
+                    # Globally blocklisted values: ignore subtest
                     if "all" in tests_per_list[driver][run_type][subname]:
                         continue
 
-                    # Nothing blocked of explicitly added.
-                    # It means that test should be on testlists
+                    if not gpu_set:
+                        gpu = "default"
+
+                    # Trivial case: fields not defined: add subtest
+                    if not gpu_set:
+                        if gpu not in testlists[driver]:
+                            testlists[driver][gpu] = {}
+
+                        if run_type not in testlists[driver][gpu]:
+                            testlists[driver][gpu][run_type] = set()
+
+                        testlists[driver][gpu][run_type].add(subname)
+                        continue
+
                     if not gpus:
                         for gpu in gpu_set:
+                            # blocked on all GPUs: ignore subtest
                             if gpu == "all":
                                 continue
 
+                            # Nothing blocked: add subtest
                             if gpu not in testlists[driver]:
                                 testlists[driver][gpu] = {}
 
-- 
2.43.0

[PATCH i-g-t v2 5/8] scripts/igt_doc.py: better handle default gpu

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

The variable "gpu" is used on several loops inside
gen_intelci_testlist(). That's confusing and error-pruned.

Instead, use "default_gpu" for the one used only for
default values.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/igt_doc.py | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/scripts/igt_doc.py b/scripts/igt_doc.py
index 1c2c7a8ef973..fd1960dac6e0 100755
--- a/scripts/igt_doc.py
+++ b/scripts/igt_doc.py
@@ -107,6 +107,7 @@ class IgtTestList(TestList):
         # Create a testlist dictionary
 
         testlists = {}
+        default_gpu = "default"
 
         for driver, run_types in tests_per_list.items():
             testlists[driver] = {}
@@ -120,18 +121,15 @@ class IgtTestList(TestList):
                     if "all" in tests_per_list[driver][run_type][subname]:
                         continue
 
-                    if not gpu_set:
-                        gpu = "default"
-
                     # Trivial case: fields not defined: add subtest
                     if not gpu_set:
-                        if gpu not in testlists[driver]:
-                            testlists[driver][gpu] = {}
+                        if default_gpu not in testlists[driver]:
+                            testlists[driver][default_gpu] = {}
 
-                        if run_type not in testlists[driver][gpu]:
-                            testlists[driver][gpu][run_type] = set()
+                        if run_type not in testlists[driver][default_gpu]:
+                            testlists[driver][default_gpu][run_type] = set()
 
-                        testlists[driver][gpu][run_type].add(subname)
+                        testlists[driver][default_gpu][run_type].add(subname)
                         continue
 
                     if not gpus:
@@ -173,7 +171,16 @@ class IgtTestList(TestList):
                             testlists[driver][gpu][run_type].add(subname)
 
                     if default_gpu_value:
-                        testlists[driver][gpu][run_type].add(subname)
+                        if default_gpu not in testlists[driver]:
+                            testlists[driver][default_gpu] = {}
+
+                        if run_type not in testlists[driver][default_gpu]:
+                            testlists[driver][default_gpu][run_type] = set()
+
+                        testlists[driver][default_gpu][run_type].add(subname)
+
+        if len(gpu_set) == 0:
+            gpu_set.add(default_gpu)
 
         return (testlists, gpu_set)
 
-- 
2.43.0

[PATCH i-g-t v2 6/8] scripts/igt_doc.py: remove some unused logic

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

There's no need to check for empty string anymore, as the logic
at gen_intelci_testlist() already ensures that missing run types
are associated with "other" The same applies to GPUs, whose
default is "default".

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/igt_doc.py | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/scripts/igt_doc.py b/scripts/igt_doc.py
index fd1960dac6e0..bf51e5f4ebb3 100755
--- a/scripts/igt_doc.py
+++ b/scripts/igt_doc.py
@@ -231,11 +231,8 @@ class IntelciTestlist:
                     pass
 
                 for testlist, subtests in names.items():
-                    if testlist == "":
-                        testlist = "other"
-                    else:
-                        testlist = re.sub(r"[\W_]+", "-", testlist).lower()
-                        testlist = re.sub(r"_+", "_", testlist)
+                    testlist = re.sub(r"[\W_]+", "-", testlist).lower()
+                    testlist = re.sub(r"_+", "_", testlist)
 
                     if not subtests:
                         if testlist != "other":
-- 
2.43.0

[PATCH i-g-t v2 7/8] scripts/igt_doc.py: use an ancillary function to simplify the code

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

The code inside gen_intelci_testlist() is too complex, and currently
buggy.

Basically, if we have two testlists with driver name + name, like
"Xe BAT" and "i915 BAT", the current logic picks just one
(randomly), which produce different results on each run.

Simplify the logic by moving the routine which fills the run_type
set to a separate function, fixing the bug.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/igt_doc.py | 72 ++++++++++++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 28 deletions(-)

diff --git a/scripts/igt_doc.py b/scripts/igt_doc.py
index 1b4f7eafd135..3a45371a27e7 100755
--- a/scripts/igt_doc.py
+++ b/scripts/igt_doc.py
@@ -18,6 +18,40 @@ import sys
 from test_list import TestList
 
 class IgtTestList(TestList):
+    def __init__(self, *args, **kwargs):
+        self.split_regex = re.compile(r",\s*")
+
+        super().__init__(*args, **kwargs)
+
+    def _get_run_type_drivers(self, run_types):
+        """
+            Ancillary routine to return drivers and run type set from
+            a run type string.
+        """
+
+        run_type_dict = {}
+        for run_type in set(self.split_regex.split(run_types)):
+            driver_set = set()
+            if not run_type:
+                run_type = "other"
+            else:
+                for driver in self.drivers:
+                    result = re.sub(r"^" + driver + r"[\W_]*", "", run_type, re.IGNORECASE)
+                    if result != run_type:
+                        driver_set = set([driver])
+                        run_type = result
+                        break
+
+            if not driver_set:
+                driver_set = set(self.drivers)
+
+            if run_type in run_type_dict:
+                run_type_dict[run_type].update(driver_set)
+            else:
+                run_type_dict[run_type] = driver_set
+
+        return run_type_dict
+
     """
         This class implements testlist generation as expected by Intel CI.
         It does that by handling test lists split by "Run type" and
@@ -50,7 +84,7 @@ class IgtTestList(TestList):
         - if "GPU excluded platform" exists, for each GPU listed on
           the list, it will block the test.
     """
-    def gen_intelci_testlist(self): #pylint: disable=R0912
+    def gen_intelci_testlist(self):
         """Return a list of gpu configs and testlists."""
 
         subtest_dict = self.expand_dictionary(True)
@@ -58,48 +92,30 @@ class IgtTestList(TestList):
         # Create a tests_per_list dict
         gpu_set = set()
         tests_per_list = {}
-        split_regex = re.compile(r",\s*")
+
+        for driver in set(self.drivers):
+            tests_per_list[driver] = {}
 
         for subname, subtest in subtest_dict.items():
             run_types = subtest.get("Run type", "other").lower()
-            drivers = set()
-            run_type_set = set()
-            for run_type in set(split_regex.split(run_types)):
 
-                for driver in self.drivers:
-                    if run_type.startswith(driver):
-                        run_type = re.sub(r"^" + driver + r"[\W_]*", "", run_type)
-                        drivers = set([driver])
-                        break
+            run_type_dict = self._get_run_type_drivers(run_types)
 
-                if not drivers:
-                    drivers.update(self.drivers)
-
-                if not run_type:
-                    run_type = "other"
-
-                run_type_set.add(run_type)
-
-            if not drivers:
-                drivers = set(self.drivers)
-
-            for driver in drivers:
-                if driver not in tests_per_list:
-                    tests_per_list[driver] = {}
-
-                for run_type in run_type_set:
+            for run_type, drivers in run_type_dict.items():
+                for driver in drivers:
                     if run_type not in tests_per_list[driver]:
                         tests_per_list[driver][run_type] = {}
+
                     if subname not in tests_per_list[driver][run_type]:
                         tests_per_list[driver][run_type][subname] = {}
 
                     if "GPU" in subtest:
-                        for gpu in split_regex.split(subtest["GPU"]):
+                        for gpu in self.split_regex.split(subtest["GPU"]):
                             gpu_set.add(gpu)
                             tests_per_list[driver][run_type][subname][gpu] = True
 
                     if "GPU excluded platform" in subtest:
-                        for gpu in split_regex.split(subtest["GPU excluded platform"]):
+                        for gpu in self.split_regex.split(subtest["GPU excluded platform"]):
                             gpu_set.add(gpu)
                             tests_per_list[driver][run_type][subname][gpu] = False
 
-- 
2.43.0

[PATCH i-g-t v2 8/8] scripts/igt_doc.py: simplify and cleanup tests_per_list logic

[Mauro Carvalho Chehab]

From: Mauro Carvalho Chehab <mchehab@kernel.org>

Re-do the logic which calculates tests_per_list dict inside
gen_intelci_testlist():

- better handle "other" runtype;

- better handle driver name discovery logic.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 scripts/igt_doc.py | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/scripts/igt_doc.py b/scripts/igt_doc.py
index bf51e5f4ebb3..1b4f7eafd135 100755
--- a/scripts/igt_doc.py
+++ b/scripts/igt_doc.py
@@ -61,36 +61,35 @@ class IgtTestList(TestList):
         split_regex = re.compile(r",\s*")
 
         for subname, subtest in subtest_dict.items():
-            run_types = subtest.get("Run type", "other")
-            run_type_set = set(split_regex.split(run_types))
-
+            run_types = subtest.get("Run type", "other").lower()
+            drivers = set()
             run_type_set = set()
             for run_type in set(split_regex.split(run_types)):
-                run_type = run_type.lower()
-
-                drivers = set(self.drivers)
 
                 for driver in self.drivers:
-                    driver = driver.lower()
-
                     if run_type.startswith(driver):
                         run_type = re.sub(r"^" + driver + r"[\W_]*", "", run_type)
                         drivers = set([driver])
                         break
 
+                if not drivers:
+                    drivers.update(self.drivers)
+
+                if not run_type:
+                    run_type = "other"
+
                 run_type_set.add(run_type)
 
+            if not drivers:
+                drivers = set(self.drivers)
+
             for driver in drivers:
                 if driver not in tests_per_list:
                     tests_per_list[driver] = {}
 
                 for run_type in run_type_set:
-                    if not run_type:
-                        run_type = "other"
-
                     if run_type not in tests_per_list[driver]:
                         tests_per_list[driver][run_type] = {}
-
                     if subname not in tests_per_list[driver][run_type]:
                         tests_per_list[driver][run_type][subname] = {}
 
-- 
2.43.0

✗ Fi.CI.BUILD: failure for Fix several issues when producing Intel CI testlists (rev2)

[Patchwork]

== Series Details ==

Series: Fix several issues when producing Intel CI testlists (rev2)
URL   : https://patchwork.freedesktop.org/series/129708/
State : failure

== Summary ==

Applying: scripts/test_list.py: fix a typo on default-if-not-excluded
Applying: scripts/igt_doc.py: better handle issues with empty testlists
Applying: scripts/igt_doc.py: handle empty run_type
Applying: scripts/igt_doc.py: only create dict if subtest will be added
Applying: scripts/igt_doc.py: better handle default gpu
Applying: scripts/igt_doc.py: remove some unused logic
Applying: scripts/igt_doc.py: use an ancillary function to simplify the code
Patch failed at 0007 scripts/igt_doc.py: use an ancillary function to simplify the code
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".