clean up manifest/addtests (#933)

* clean up manifest/addtests

* fix githook

* attempt to force failure on bad linting

* use correct linter

* test lint again

* test lint again

* now test lint passes

* don't require future tests for chooser

* mark zip type unstable

Author: ben-c-at-moz

addtests.py

@@ -1,4 +1,5 @@
 import os
+import sys
 
 from manifests import testkey
 
@@ -9,4 +10,7 @@
 
 if __name__ == "__main__":
     manifest = testkey.TestKey(TEST_KEY)
-    manifest.addtests()
+    if len(sys.argv) > 1 and sys.argv[1] == "-q":
+        manifest.addtests(interactive=False)
+    else:
+        manifest.addtests()

choose_ci_set.py

@@ -220,6 +220,7 @@ def dedupe(run_list: list) -> list:
     # Dedupe just in case
     if SLASH == "\\":
         run_list = [entry.replace("/", SLASH) for entry in run_list]
+    run_list = [entry for entry in run_list if os.path.exists(entry)]
     run_list = manifest.filter_filenames_by_pass(run_list)
     run_list = dedupe(run_list)
     run_list = [entry for entry in run_list if os.path.exists(entry.split("::")[0])]

make_manifest.py

@@ -27,7 +27,7 @@ address_bar_and_search:
   test_addressbar_bookmarks_when_history_disabled:
     result: pass
     splits:
-      - functional1
+    - functional1
   test_addressbar_search_engine_keywords:
     result: pass
     splits:
@@ -363,10 +363,7 @@ downloads:
     splits:
     - smoke
   test_add_zip_type:
-    result:
-      linux: unstable
-      mac: unstable
-      win: unstable
+    result: unstable
     splits:
     - smoke
     - ci-extended

manifests/incident.yaml

@@ -1,11 +1,14 @@
 import os
 import platform
+import re
+import sys
 from copy import deepcopy
 
 import yaml
 
 NUM_FUNCTIONAL_SPLITS = 1
 MAX_DEPTH = 5
+SUITE_TUPLE_RE = re.compile(r'\s+return \("S(\d+)", ?".*"\)')
 
 
 def sysname():
@@ -196,20 +199,54 @@ def gather_split(self, split_name, pass_only=True):
 
         return test_filenames
 
-    def addtests(self):
+    def get_valid_suites_in_split(self, split, suite_numbers=False):
+        filenames = self.gather_split(split)
+        suite_dirs = []
+        for filename in filenames:
+            suite_dir = filename.split(os.path.sep)[1]
+            if suite_dir not in suite_dirs:
+                suite_dirs.append(suite_dir)
+
+        if not suite_numbers:
+            return suite_dirs
+        else:
+            suite_nums = []
+            for suite_dir in suite_dirs:
+                with open(os.path.join("tests", suite_dir, "conftest.py")) as fh:
+                    lines = fh.readlines()
+                suite_flag = False
+                for line in lines:
+                    if suite_flag:
+                        m = SUITE_TUPLE_RE.search(line)
+                        if m and len(m.groups()) > 0:
+                            suite_num = m.group(1)
+                        if suite_num not in suite_nums:
+                            suite_nums.append(suite_num)
+                        break
+            return suite_nums
+
+    def addtests(self, interactive=True):
         newkey = deepcopy(self.manifest)
         for root, _, files in os.walk(self.test_root):
             for f in files:
                 suite = root.split(os.path.sep)[1]
+                if not (f.startswith("test_") and f.endswith(".py")):
+                    continue
                 testfile = f.replace(".py", "")
                 if suite not in newkey:
+                    if not interactive:
+                        sys.exit(
+                            "Please run `python addtests.py` from your command prompt."
+                        )
                     if ask_question(f"Found suite {suite}. Add it to key? "):
                         newkey[suite] = {}
 
                 resplit = False
                 if testfile not in newkey[suite]:
-                    if not testfile.startswith("test_"):
-                        continue
+                    if not interactive:
+                        sys.exit(
+                            "Please run `python addtests.py` from your command prompt."
+                        )
                     if ask_question(f"Found test {suite}/{testfile}. Add it to key? "):
                         newkey[suite][testfile] = {"result": "pass"}
 
@@ -229,17 +266,17 @@ def addtests(self):
                     resplit = True
 
                 if resplit:
-                    split = ask_open_question(
-                        "What split is this test assigned to? (One only) "
-                    )
-                    newkey[suite][testfile]["splits"] = [split]
-
                     if ask_question(
                         "Should this test run in a Scheduled Functional split? "
                         "(Say no if unsure.) "
                     ):
-                        newkey[suite][testfile]["splits"].append("functional1")
+                        newkey[suite][testfile]["splits"] = ["functional1"]
                         self.rebalance_functionals()
+                    else:
+                        split = ask_open_question(
+                            "What split is this test assigned to? (One only) "
+                        )
+                        newkey[suite][testfile]["splits"] = [split]
 
                     print(
                         f"Test will be added to {split} in {self.manifest_file}. "

manifests/key.yaml

@@ -5,6 +5,7 @@
 import sys
 
 from choose_l10n_ci_set import select_l10n_mappings
+from manifests.testkey import TestKey
 from modules import taskcluster as tc
 from modules import testrail as tr
 from modules.testrail import TestRail
@@ -17,6 +18,7 @@
     "[{channel} {major}] {plan}Automated testing {major}.{minor}b{beta}-build{build}"
 )
 PLAN_NAME_RE = re.compile(r"\[(\w+) (\d+)\]")
+TEST_KEY_LOCATION = os.path.join("manifests", "key.yaml")
 CONFIG_GROUP_ID = 95
 TESTRAIL_FX_DESK_PRJ = 17
 TC_EXECUTION_TEMPLATE = "https://firefox-ci-tc.services.mozilla.com/tasks/%TASK_ID%/runs/%RUN_ID%/logs/live/public/logs/live.log"
@@ -267,24 +269,30 @@ def reportable(platform_to_test=None):
         # Only report when there is a new beta without a reported plan or if the selected split is not completely reported.
         return covered_mappings < expected_mappings
     else:
-        covered_suites = 0
+        covered_suites = []
         for entry in plan_entries:
             for run_ in entry.get("runs"):
                 if run_.get("config") and platform in run_.get("config"):
-                    covered_suites += 1
+                    covered_suites.append(str(run_.get("suite_id")))
 
-        num_suites = 0
-        for test_dir_name in os.listdir("tests"):
-            test_dir = os.path.join("tests", test_dir_name)
-            if os.path.isdir(test_dir) and not os.path.exists(
-                os.path.join(test_dir, "skip_reporting")
-            ):
-                num_suites += 1
-
-        logging.warning(
-            f"Potentially matching run found for {platform}, may be reportable. ({covered_suites} out of {num_suites} suites already reported.)"
+        if not os.environ.get("STARFOX_SPLIT"):
+            sys.exit("No split selected")
+        manifest = TestKey()
+        expected_suites = manifest.get_valid_suites_in_split(
+            os.environ["STARFOX_SPLIT"]
         )
-        return covered_suites + 1 < num_suites
+
+        uncovered_suites = set(expected_suites) - set(covered_suites)
+        if len(uncovered_suites):
+            suite_names = [
+                s.get("name")
+                for s in tr_session.get_suites(TESTRAIL_FX_DESK_PRJ)
+                if str(s.get("id")) in uncovered_suites
+            ]
+            print("Coverage not found for the following suites:")
+            print("\t-" + "\n\t-".join(suite_names))
+
+        return not uncovered_suites
 
 
 def testrail_init() -> TestRail: