address comments

tests/spread-filter/config.yaml → tests/spread-filter/rules.yaml

@@ -1,6 +1,6 @@
 rules:
-  rule_tests:
-    from: 
+  tests:
+    from:
       - tests/main/.*
       - tests/core/.*
       - tests/nested/.*
@@ -12,22 +12,22 @@ rules:
       - tests/unit/.*
       - tests/upgrade/.*
     to: [$SELF]
-  
-  rule_nested:
+
+  nested:
     from: [tests/lib/nested.sh]
     to: [tests/nested/]
 
-  rule_unit:
+  unit:
     from: [.*_test.go]
-    to: 
+    to:
      - tests/unit/go
      - tests/unit/c-unit-tests-clang
      - tests/unit/c-unit-tests-gcc
 
-  rule_docs:
+  docs:
     from: [.*.md]
     to: [$NONE]
 
-  rule_rest: 
+  rest:
     from: [.*]
     to: [tests/]

tests/spread-filter/task.yaml

@@ -1,8 +1,8 @@
 summary: test for the spread-filter utility
 
 details: |
-    Check the spread-filter utility retrieves properly the list of tests
-    to execute following the rules defined for the project and based on a
+    Check that the spread-filter utility properly retrieves the list of tests
+    to execute, following the rules defined for the project and based on a
     set of changes.
 
 backends: [google]
@@ -14,80 +14,78 @@ execute: |
     spread-filter -h | MATCH 'usage: spread-filter'
 
     # Scenario: 1 test changed
-    spread-filter -r config.yaml -s ubuntu-22.04-64 -b google -c tests/main/test1/task.yaml > res
+    spread-filter -r rules.yaml -p google:ubuntu-22.04-64 -c tests/main/test1/task.yaml > res
     MATCH "^google:ubuntu-22.04-64:tests/main/test1$" < res
 
     # Scenario: 2 tests changed
-    spread-filter -r config.yaml -s ubuntu -b google -c tests/main/test1/task.yaml -c tests/core/test1/task.yaml > res
+    spread-filter -r rules.yaml -p google:ubuntu -c tests/main/test1/task.yaml -c tests/core/test1/task.yaml > res
     MATCH "^google:ubuntu:tests/main/test1 google:ubuntu:tests/core/test1$" < res
 
     # Scenario: 2 files on the same test dir
-    spread-filter -r config.yaml -s ubuntu -b google -c tests/core/test2/task.yaml -c tests/core/test2/otherfile.py > res
+    spread-filter -r rules.yaml -p google:ubuntu -c tests/core/test2/task.yaml -c tests/core/test2/otherfile.py > res
     MATCH "^google:ubuntu:tests/core/test2$" < res
 
     # Scenario: 2 files on the same test dir but 1 file does not exist (deleted)
-    spread-filter -r config.yaml -s ubuntu -b google -c tests/core/test2/task.yaml -c tests/core/test2/noexist > res
+    spread-filter -r rules.yaml -p google:ubuntu -c tests/core/test2/task.yaml -c tests/core/test2/noexist > res
     MATCH "^google:ubuntu:tests/core/test2$" < res
 
     # Scenario: 3 files on the same test dir but 2 files are in subdir and 1 file does not exist (deleted)
-    spread-filter -r config.yaml -s ubuntu -b google -c tests/main/test2/otherfile -c tests/main/test2/snap/bin/sh -c tests/main/test2/snap/meta/noexist > res
+    spread-filter -r rules.yaml -p google:ubuntu -c tests/main/test2/otherfile -c tests/main/test2/snap/bin/sh -c tests/main/test2/snap/meta/noexist > res
     MATCH "^google:ubuntu:tests/main/test2$" < res
 
     # Scenario: 1 file and 1 subdirectory changed
-    spread-filter -r config.yaml -s ubuntu -b google -c tests/main/test2/otherfile -c tests/main/test2/snap > res
+    spread-filter -r rules.yaml -p google:ubuntu -c tests/main/test2/otherfile -c tests/main/test2/snap > res
     MATCH "^google:ubuntu:tests/main/test2$" < res
 
     # Scenario: 1 test and other file changed (check just the big run survives)
-    spread-filter -r config.yaml -s ubuntu -b google -c tests/main/test2/task.yaml -c source/lib1/lib1 > res
+    spread-filter -r rules.yaml -p google:ubuntu -c tests/main/test2/task.yaml -c source/lib1/lib1 > res
     MATCH "^google:ubuntu:tests/$" < res
 
     # Scenario: 2 unit tests changed
-    spread-filter -r config.yaml -s ubuntu -b google -c source/lib1/lib1_test.go -c source/lib2/lib2_test.go > res
+    spread-filter -r rules.yaml -p google:ubuntu -c source/lib1/lib1_test.go -c source/lib2/lib2_test.go > res
     MATCH "^google:ubuntu:tests/unit/c-unit-tests-clang google:ubuntu:tests/unit/c-unit-tests-gcc google:ubuntu:tests/unit/go$" < res
 
     # Scenario: 1 unit tests and 1 test changed
-    spread-filter -r config.yaml -s ubuntu -b google -c source/lib1/lib1_test.go -c tests/main/test1/noexist.go > res
+    spread-filter -r rules.yaml -p google:ubuntu -c source/lib1/lib1_test.go -c tests/main/test1/noexist.go > res
     MATCH "^google:ubuntu:tests/unit/c-unit-tests-clang google:ubuntu:tests/unit/c-unit-tests-gcc google:ubuntu:tests/main/test1 google:ubuntu:tests/unit/go$" < res
 
     # Scenario: a doc file changed
-    spread-filter -r config.yaml -s ubuntu -b google -c README.md > res
+    spread-filter -r rules.yaml -p google:ubuntu -c README.md > res
     test "$(wc -w < res)" -eq 0
 
     # Scenario: a doc file which does not exist changed (deleted)
-    spread-filter -r config.yaml -s ubuntu -b google -c DELETED.md > res
+    spread-filter -r rules.yaml -p google:ubuntu -c DELETED.md > res
     test "$(wc -w < res)" -eq 0
 
     # Scenario: a doc file and a the nested library changed
-    spread-filter -r config.yaml -s ubuntu -b google -c README.md -c tests/lib/nested.sh > res
+    spread-filter -r rules.yaml -p google:ubuntu -c README.md -c tests/lib/nested.sh > res
     MATCH "^google:ubuntu:tests/nested/$" < res
     
     # Scenario: 1 test and also another which is substring of previous one changed
-    spread-filter -r config.yaml -s ubuntu -b google -c README.md -c tests/main/test1/task.yaml -c tests/main/test1b/task.yaml > res
+    spread-filter -r rules.yaml -p google:ubuntu -c README.md -c tests/main/test1/task.yaml -c tests/main/test1b/task.yaml > res
     MATCH "^google:ubuntu:tests/main/test1b google:ubuntu:tests/main/test1$" < res
 
     # Scenario: a file in the test suite but not in a test dir changed
-    spread-filter -r config.yaml -s ubuntu -b google -c tests/core/other/lib1 > res
+    spread-filter -r rules.yaml -p google:ubuntu -c tests/core/other/lib1 > res
     test "$(wc -w < res)" -eq 0
 
     # Scenario: the nested lib and a nested test are changed (check the test is cleaned)
-    spread-filter -r config.yaml -s ubuntu -b google -c tests/lib/nested.sh -c tests/nested/test1/task.yaml > res
+    spread-filter -r rules.yaml -p google:ubuntu -c tests/lib/nested.sh -c tests/nested/test1/task.yaml > res
     MATCH "^google:ubuntu:tests/nested/$" < res
 
     # Scenario: the nested lib, a source file and a nested test are changed (check just tests/ survives)
-    spread-filter -r config.yaml -s ubuntu -b google -c tests/lib/nested.sh -c source/lib1/lib1 -c tests/nested/test1/task.yaml > res
+    spread-filter -r rules.yaml -p google:ubuntu -c tests/lib/nested.sh -c source/lib1/lib1 -c tests/nested/test1/task.yaml > res
     MATCH "^google:ubuntu:tests/$" < res
 
     # Scenario: check -v parameter works
-    spread-filter -r config.yaml -s ubuntu -b google -c tests/lib/nested.sh -v > res
+    spread-filter -r rules.yaml -p google:ubuntu -c tests/lib/nested.sh -v > res
     MATCH "cleaning executions" < res
 
     # Scenario: check mandatory parameters
     ARG_TEXT="spread-filter: error: the following arguments are required"
-    spread-filter -r config.yaml -b google -c tests/lib/nested.sh 2>&1 | MATCH "$ARG_TEXT"
-    spread-filter -r config.yaml -s ubuntu -c tests/lib/nested.sh 2>&1 | MATCH "$ARG_TEXT"
-    spread-filter -s ubuntu -b google -c tests/lib/nested.sh 2>&1 | MATCH "$ARG_TEXT"
+    spread-filter -r rules.yaml -c tests/lib/nested.sh 2>&1 | MATCH "$ARG_TEXT"
+    spread-filter -p google:ubuntu -c tests/lib/nested.sh 2>&1 | MATCH "$ARG_TEXT"
 
-    # Scenario: check the config rules file
+    # Scenario: check the rules file
     FILE_TEXT="spread-filter: rules file 'noexist.yaml' does not exist"
-    spread-filter -r noexist.yaml -s ubuntu -b google -c tests/lib/nested.sh 2>&1 | MATCH "$FILE_TEXT"
-  
+    spread-filter -r noexist.yaml -p google:ubuntu -c tests/lib/nested.sh 2>&1 | MATCH "$FILE_TEXT"

tests/tests.pkgs/task.yaml

@@ -51,8 +51,8 @@ execute: |
     tests.pkgs noexist test-snapd-pkg-1 2>&1 | MATCH 'tests.pkgs: unknown command noexist'
     tests.pkgs -install test-snapd-pkg-1 2>&1 | MATCH 'tests.pkgs: unknown option -install'
 
-    # Install the test pkg and check it is installed and query it
-    # run this just on amd64 architecture and skip trusty
+    # Install the test package, verify its installation and query it
+    # Run this only on amd64 and skip Trusty
     if ( os.query is-debian || os.query is-ubuntu-ge 1604 ) && os.query is-pc-amd64 ; then
       dpkg --add-architecture i386
       tests.pkgs install test-snapd-pkg-3

utils/spread-filter

@@ -42,10 +42,9 @@ class ExecutionManager:
     SELF = "$SELF"
     NONE = "$NONE"
 
-    def __init__(self, rules_file: str, system: str, backend: str, verbose: bool):
+    def __init__(self, rules_file: str, prefix: str, verbose: bool):
         self.verbose = verbose
-        self.system = system
-        self.backend = backend
+        self.prefix = prefix
         with open(rules_file) as f:
             rules_map = yaml.safe_load(f)
         self.rules = ExecutionRules(self.from_rules_map(rules_map))
@@ -74,37 +73,37 @@ class ExecutionManager:
     def _get_execution_param(self, exec_paths: list[str]) -> list[str]:
         all_params = []
         for exec_path in exec_paths:
-            all_params.append(self.backend + ":" + self.system + ":" + exec_path)
+            all_params.append(self.prefix + ":" + exec_path)
         return all_params
 
     def _calc_self(self, change_path: str) -> str:
         if self.verbose:
             print("calculating self for {}".format(change_path))
 
-        # if the change is a directory which has changed
+        # If the change is a directory which has changed
         if os.path.isdir(change_path):
             if self._is_test_dir(change_path):
                 return self._get_test_exec_dir(change_path)
             else:
                 test_dir = self._get_parent_with_task(change_path)
                 return self._get_test_exec_dir(test_dir)
-        # if the change is a file which has changed
+        # If the change is a file which has changed
         elif os.path.isfile(change_path):
-            # if the change is a task.yaml
+            # If the change is a task.yaml
             if self._is_task_file(change_path):
                 return self._get_test_exec_dir(os.path.dirname(change_path))
 
-            # if the change is a file in the same dir than a task.yaml
+            # If the change is a file in the same dir than a task.yaml
             change_dir = os.path.dirname(change_path)
             if self._is_test_dir(change_dir):
                 return self._get_test_exec_dir(change_dir)
-            # if the change is a file which is not in the same dir than a task.yaml
+            # If the change is a file which is not in the same dir than a task.yaml
             else:
                 test_dir = self._get_parent_with_task(change_path)
                 return self._get_test_exec_dir(test_dir)
-        # if the change is deleted file/dir
+        # If the change is deleted file/dir
         else:
-            # if a task.yaml has been deleted
+            # If a task.yaml has been deleted
             if self._is_task_file(change_path):
                 return ""
             else:
@@ -143,21 +142,21 @@ class ExecutionManager:
                         )
                     )
 
-                # if the shorter executions is a test, we continue
+                # If the shorter executions is a test, we continue
                 if not short_execution.endswith("/"):
                     if long_execution == short_execution:
                         discard_execution = True
                         break
-                # if the shorter execution is a either suite or group of suites
+                # If the shorter execution is a either suite or group of suites
                 else:
-                    # if the long executions starts with short execution means
+                    # If the long executions starts with short execution means
                     # the long execution is included in the short one and it can
                     # be discarded
                     if long_execution.startswith(short_execution):
                         discard_execution = True
                         break
 
-            # take decision after all shorter executions have been compared
+            # Take decision after all shorter executions have been compared
             if not discard_execution:
                 final_executions.append(long_execution)
 
@@ -175,11 +174,11 @@ class ExecutionManager:
                     self_execution = self._calc_self(change)
                     if self_execution != "":
                         all_executions.append(self_execution)
-                # When the tests to run as tagged as NONE means that no tests
+                # When the tests to run are tagged as NONE means that no tests
                 # have to be executed
                 elif execution == self.NONE:
                     continue
-                # Otherwise, it is executed what is defined in to (literal)
+                # Otherwise, what is defined in to (literal) is executed.
                 else:
                     all_executions.append(self._get_test_exec_dir(execution))
 
@@ -188,13 +187,13 @@ class ExecutionManager:
 
 
 def _make_parser() -> argparse.ArgumentParser:
-    parser = argparse.ArgumentParser(description="spread shellcheck helper")
+    parser = argparse.ArgumentParser(description="spread filter helper")
     parser.add_argument("-r", "--rules-file", required=True, help="Rules file")
     parser.add_argument(
-        "-s", "--system", required=True, help="System used for executing the tests"
-    )
-    parser.add_argument(
-        "-b", "--backend", required=True, help="Backend used for executing the tests"
+        "-p",
+        "--prefix",
+        required=True,
+        help="Backend and system (BACKEND:SYSTEM) used as prefix in the output",
     )
     parser.add_argument(
         "-c", "--change", action="append", default=[], help="File that changed"
@@ -216,8 +215,6 @@ if __name__ == "__main__":
         print("spread-filter: rules file '{}' does not exist".format(args.rules_file))
         sys.exit(1)
 
-    execution_manager = ExecutionManager(
-        args.rules_file, args.system, args.backend, args.verbose
-    )
+    execution_manager = ExecutionManager(args.rules_file, args.prefix, args.verbose)
     execution_list = execution_manager.get_executions(args.change)
     print(" ".join(map(str, execution_list)))