Ben/new manifests

.github/workflows/smoke.yml

@@ -93,6 +93,10 @@ jobs:
       - name: Download Executable
         if: ${{ inputs.win_installer_link }}
         run: |
+          $env_contents = @"
+          STARFOX_MANIFEST='.\manifests\incident.yaml'
+          "@
+          New-Item -Name .env -Value $env_contents -ItemType File -Force
           Invoke-WebRequest -Uri ${{ inputs.win_installer_link }} -OutFile "${{ github.workspace }}\setup.exe"
           New-Item -ItemType Directory -Path "C:\Program Files\Custom Firefox" -Force
         shell: pwsh
@@ -108,7 +112,7 @@ jobs:
           $env:FX_EXECUTABLE = "C:\Program Files\Custom Firefox\firefox.exe"
           Start-Process -FilePath $env:FX_EXECUTABLE -ArgumentList "--version" -Wait -NoNewWindow
           pipenv run python choose_ci_set.py
-          pipenv run pytest $(cat selected_tests)
+          pipenv run pytest $(cat selected_tests) --geckodriver=geckodriver.exe
           $env:TEST_EXIT_CODE = $LASTEXITCODE
           mv artifacts artifacts-win || true
           exit $env:TEST_EXIT_CODE
@@ -121,7 +125,7 @@ jobs:
           mv ./ci_pyproject_headed.toml ./pyproject.toml;
           $env:FX_EXECUTABLE = "C:\Program Files\Custom Firefox\firefox.exe"
           pipenv run python choose_ci_set.py
-          pipenv run pytest $(cat selected_tests)
+          pipenv run pytest $(cat selected_tests) --geckodriver=geckodriver.exe
           $env:TEST_EXIT_CODE = $LASTEXITCODE
           rm artifacts/assets -r -Force
           Get-ChildItem -Path "artifacts" | ForEach-Object {
@@ -165,6 +169,7 @@ jobs:
         if: ${{ inputs.mac_installer_link }}
         run: |
           echo "MANUAL='true'" >> "$GITHUB_ENV";
+          echo "STARFOX_MANIFEST=manifests/incident.yaml" >> "$GITHUB_ENV"
           echo "Running smoke tests on supplied executable";
       - name: Install dependencies
         run: |
@@ -228,6 +233,7 @@ jobs:
           MANUAL_DOWNLOAD_LINK: ${{ inputs.linux_tarball_link }}
         run: |
           echo "MANUAL='true'" >> "$GITHUB_ENV";
+          echo "STARFOX_MANIFEST=manifests/incident.yaml" >> "$GITHUB_ENV"
           echo "Running smoke tests on supplied executable";
           sudo apt install gnome-screenshot
           uname -m;

README.md

@@ -127,6 +127,48 @@ You may find that if you are re-running all previously executed test runs that y
 "Session is not reportable." If you wish to overwrite a previously reported session, add
 `REPORTABLE=true` to your environment.
 
+### Marking Tests For Skipping
+
+The file `manifests/key.yaml` is the single source of truth for whether a test exists, and whether it
+should or should not be skipped. The other files in `manifests/` are test lists. The schema for the key
+file is:
+
+```yaml
+suite_name_which_is_the_folder_under_tests:
+  test_file_without_the_dot_py: pass
+address_bar_and_search:
+  test_thing_does_stuff:
+    test_a_subtest_inside_this_file: pass
+  test_another_thing:
+    mac: pass
+    win: unstable
+    linux: pass
+tabs:
+  test_tab_says_hi:
+    test_clever_subtest_name:
+      mac: unstable
+      win: pass
+      linux: pass
+```
+
+Any value other than `pass` will skip the test or subtest (for the given OS if applicable). It is good
+practice to keep non-pass values limited. Good values are `unstable`, `deprecated`, `out-of-scope` etc.
+**Do not use `fail` for tests you wish to see pass again one day.** Future work will include testing
+items marked `fail` as xfail, and may implement `strict_xfail`, which will throw if tests pass.
+
+The test lists in `manifests/` have the following schema:
+
+```yaml
+suite_name_a:
+- test_name_b
+- test_name_c
+suite_name_d:
+- test_name_e
+```
+
+We currently do not assume that test lists need to identify individual test functions inside test
+files ("subtests"), as TestRail reporting is on the testfile level.
+
 ### Manual Execution of Smoke Tests
 
 To run the smoke tests manually against an arbitrary version of Firefox **where the installer or (for Linux)

choose_ci_set.py

@@ -1,24 +1,19 @@
 import os
+import platform
 import re
 import sys
 from subprocess import check_output
 
-import pytest
+import yaml
 
-SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
-CI_MARK = "@pytest.mark.ci"
+CI_MANIFEST = "manifests/ci.yaml"
+MANIFEST_KEY = "manifests/key.yaml"
+SUPPORTED_OSES = ["mac", "win", "linux"]
 HEADED_MARK = "@pytest.mark.headed"
+MIN_RUN_SIZE = 7
 OUTPUT_FILE = "selected_tests"
-
-
-class CollectionPlugin:
-    """Mini plugin to get test names"""
-
-    def __init__(self):
-        self.tests = []
-
-    def pytest_report_collectionfinish(self, items):
-        self.tests = [item.nodeid for item in items]
+SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
+SLASH = "/" if "/" in SCRIPT_DIR else "\\"
 
 
 def snakify(pascal: str) -> str:
@@ -72,7 +67,7 @@ def get_tests_by_model(
     return matching_tests
 
 
-def dedupe(run_list: list, slash: str) -> list:
+def dedupe(run_list: list) -> list:
     """For a run list, remove entries that are covered by more general entries."""
     run_list = list(set(run_list))
     dotslashes = []
@@ -87,7 +82,7 @@ def dedupe(run_list: list, slash: str) -> list:
             dotslashes.append(i)
 
     for dotslash in dotslashes:
-        run_list[dotslash] = f".{slash}{run_list[dotslash]}"
+        run_list[dotslash] = f".{SLASH}{run_list[dotslash]}"
 
     for i, entry_a in enumerate(run_list):
         for j, entry_b in enumerate(run_list):
@@ -105,7 +100,64 @@ def dedupe(run_list: list, slash: str) -> list:
     return run_list
 
 
+def sysname():
+    sys_platform = platform.system().lower()
+    if sys_platform.startswith("darwin"):
+        return "mac"
+    elif sys_platform.startswith("win"):
+        return "win"
+    elif sys_platform.startswith("linux"):
+        return "linux"
+    raise OSError("Unsupported system.")
+
+
+def convert_manifest_to_list(manifest_loc):
+    manifest = yaml.safe_load(open(manifest_loc))
+    mkey = yaml.safe_load(open(MANIFEST_KEY))
+    toplevel = [".", "tests"]
+    tests = []
+    if manifest:
+        print(f"Reading {manifest_loc}")
+    for suite in manifest:
+        if suite not in mkey:
+            print(f"{suite} not in {MANIFEST_KEY}")
+            continue
+
+        for testfile in manifest[suite]:
+            addtest = False
+            test_name = f"{testfile}.py"
+            if testfile not in mkey[suite]:
+                print(f"{suite}/{testfile} not in {MANIFEST_KEY}::{suite}")
+                continue
+            if mkey[suite][testfile] == "pass":
+                addtest = True
+            elif isinstance(mkey[suite][testfile], dict):
+                if any([x in mkey[suite][testfile] for x in SUPPORTED_OSES]):
+                    if mkey[suite][testfile][sysname()] == "pass":
+                        addtest = True
+                else:
+                    for subtest in mkey[suite][testfile]:
+                        if mkey[suite][testfile][subtest] == "pass":
+                            test_name = f"{test_name}::{subtest}"
+                            addtest = True
+                        elif isinstance(mkey[suite][testfile][subtest], dict):
+                            if sysname() in mkey[suite][testfile][subtest]:
+                                if mkey[suite][testfile][subtest][sysname()] == "pass":
+                                    test_name = f"{test_name}::{subtest}"
+                                    addtest = True
+
+            if addtest:
+                test_to_add = SLASH.join(toplevel + [suite, test_name])
+                assert os.path.exists(test_to_add.split("::")[0]), (
+                    f"{test_to_add} could not be found"
+                )
+                tests.append(test_to_add)
+                addtest = False
+    return tests
+
+
 if __name__ == "__main__":
+    print("Selecting test set...")
     if os.path.exists(".env"):
         with open(".env") as fh:
             contents = fh.read()
@@ -114,13 +166,20 @@ def dedupe(run_list: list, slash: str) -> list:
             if "RUN_ALL='true'" in contents:
                 os.environ["MANUAL"] = "true"
 
-    if os.environ.get("TESTRAIL_REPORT") or os.environ.get("MANUAL"):
-        # Run all tests if this is a scheduled beta or a manual run
+    if os.environ.get("TESTRAIL_REPORT"):
+        # Run all tests if this is a scheduled run
+        run_list = convert_manifest_to_list("manifests/smoke.yaml")
+        run_list = dedupe(run_list)
         with open(OUTPUT_FILE, "w") as fh:
-            fh.write("tests")
+            fh.write("\n".join(run_list))
             sys.exit(0)
 
-    slash = "/" if "/" in SCRIPT_DIR else "\\"
+    if os.environ.get("STARFOX_MANIFEST"):
+        run_list = convert_manifest_to_list(os.environ["STARFOX_MANIFEST"])
+        run_list = dedupe(run_list)
+        with open(OUTPUT_FILE, "w") as fh:
+            fh.write("\n".join(run_list))
+            sys.exit(0)
 
     re_obj = {
         "test_re_string": r".*/.*/test_.*\.py",
@@ -130,7 +189,7 @@ def dedupe(run_list: list, slash: str) -> list:
         "class_re_string": r"\s*class (\w+)[(A-Za-z0-9_)]*:",
     }
     for k in list(re_obj.keys()):
-        if slash == "\\":
+        if SLASH == "\\":
             re_obj[k] = re_obj.get(k).replace("/", r"\\")
         short_name = "_".join(k.split("_")[:-1])
         re_obj[short_name] = re.compile(re_obj.get(k))
@@ -140,17 +199,19 @@ def dedupe(run_list: list, slash: str) -> list:
     committed_files = (
         check_output(["git", "--no-pager", "diff", "--name-only", "origin/main"])
         .decode()
-        .replace("/", slash)
+        .replace("/", SLASH)
         .splitlines()
     )
 
     main_conftest = "conftest.py"
     base_page = os.path.join("modules", "page_base.py")
 
     if main_conftest in committed_files or base_page in committed_files:
-        # Run all the tests (no files as arguments) if main conftest or basepage changed
+        # Run smoke tests if main conftest or basepage changed
+        run_list = convert_manifest_to_list("manifests/smoke.yaml")
+        run_list = dedupe(run_list)
         with open(OUTPUT_FILE, "w") as fh:
-            fh.write("tests")
+            fh.write("\n".join(run_list))
         sys.exit(0)
 
     all_tests = []
@@ -164,13 +225,6 @@ def dedupe(run_list: list, slash: str) -> list:
                     lines = fh.readlines()
                     test_paths_and_contents[this_file] = "".join(lines)
 
-    p = CollectionPlugin()
-    pytest.main(["--collect-only", "-m", "ci", "-s"], plugins=[p])
-    ci_paths = [f".{slash}{test}" for test in p.tests]
-
-    # Dedupe just in case
-    ci_paths = list(set(ci_paths))
-
     changed_suite_conftests = [
         f for f in committed_files if re_obj.get("suite_conftest_re").match(f)
     ]
@@ -184,7 +238,7 @@ def dedupe(run_list: list, slash: str) -> list:
 
     if changed_suite_conftests:
         run_list = [
-            "." + slash + os.path.join(*suite.split(slash)[-3:-1])
+            "." + SLASH + os.path.join(*suite.split(SLASH)[-3:-1])
             for suite in changed_suite_conftests
         ]
 
@@ -212,7 +266,7 @@ def dedupe(run_list: list, slash: str) -> list:
             found = False
             for file in run_list:
                 # Don't add if already exists in suite changes
-                pieces = file.split(slash)
+                pieces = file.split(SLASH)
                 if len(pieces) == 3 and pieces[-1] in changed_test:
                     found = True
 
@@ -223,15 +277,19 @@ def dedupe(run_list: list, slash: str) -> list:
                 run_list.append(changed_test)
 
     if not run_list:
+        ci_paths = convert_manifest_to_list(CI_MANIFEST)
+        ci_paths = dedupe(ci_paths)
         with open(OUTPUT_FILE, "w") as fh:
             fh.write("\n".join(ci_paths))
-    else:
+        sys.exit(0)
+
+    if len(run_list) < MIN_RUN_SIZE:
         run_list.extend(ci_paths)
 
-        # Dedupe just in case
-        if slash == "\\":
-            run_list = [entry.replace("/", slash) for entry in run_list]
-        run_list = dedupe(run_list, slash)
-        run_list = [entry for entry in run_list if os.path.exists(entry.split("::")[0])]
-        with open(OUTPUT_FILE, "w") as fh:
-            fh.write("\n".join(run_list))
+    # Dedupe just in case
+    if SLASH == "\\":
+        run_list = [entry.replace("/", SLASH) for entry in run_list]
+    run_list = dedupe(run_list)
+    run_list = [entry for entry in run_list if os.path.exists(entry.split("::")[0])]
+    with open(OUTPUT_FILE, "w") as fh:
+        fh.write("\n".join(run_list))

choose_test_channel.py

@@ -3,7 +3,7 @@
 import sys
 from subprocess import check_output
 
-ALL_CHANNELS = ["smoke", "l10n"]
+ALL_CHANNELS = ["smoke", "l10n", "functional"]
 SCRIPT_DIR = os.path.dirname(os.path.abspath(__file__))
 SLASH = "/" if "/" in SCRIPT_DIR else "\\"
 
@@ -29,6 +29,10 @@
     ]
 )
 
+if len(sys.argv) > 1:
+    print(sys.argv[1:])
+    sys.exit(0)
+
 check_output(["git", "fetch", "--quiet", "--depth=1", "origin", "main"])
 
 committed_files = (

ci_pyproject.toml

@@ -5,17 +5,14 @@ log_cli_level = "warn"
 markers = [
     "audio: test is reliant on audio",
     "headed: test must run in headed mode (e.g. uses pynput)",
-    "incident: incident smoke tests",
-    "unstable: temporary mark for unstable tests",
     "slow: test is clocked at more than 30s on modern machines",
-    "ci: basic tests to run in ci",
     "locale_de: tests run in DE locale versions",
     "locale_fr: tests run in FR locale versions",
     "locale_gb: tests run in GB locale versions",
     "noxvfb: tests that should not run in xvfb sessions"
 ]
 
-addopts = "-vs --ci --run-headless --json-report --json-report-file artifacts/report.json --reruns 3 -n auto --reruns-delay 3 -m 'not incident and not unstable and not headed' --html=artifacts/report.html"
+addopts = "-vs --ci --run-headless --json-report --json-report-file artifacts/report.json --reruns 3 -n auto --reruns-delay 3 -m 'not headed' --html=artifacts/report.html"
 
 [tool.ruff]
 target-version = "py310"

ci_pyproject_headed.toml

@@ -5,16 +5,13 @@ log_cli_level = "warn"
 markers = [
     "audio: test is reliant on audio",
     "headed: test must run in headed mode (e.g. uses pynput)",
-    "incident: incident smoke tests",
-    "unstable: temporary mark for unstable tests",
     "slow: test is clocked at more than 30s on modern machines",
-    "ci: basic tests to run in ci",
     "locale_de: tests run in DE locale versions",
     "locale_fr: tests run in FR locale versions",
     "locale_gb: tests run in GB locale versions",
     "noxvfb: tests that should not run in xvfb sessions"
 ]
-addopts = "-vs --ci --json-report --json-report-file artifacts/report_headed.json --reruns 3 --reruns-delay 2 -m 'not incident and not unstable and headed' --html=artifacts/report_headed.html"
+addopts = "-vs --ci --json-report --json-report-file artifacts/report_headed.json --reruns 3 --reruns-delay 2 -m 'headed' --html=artifacts/report_headed.html"
 
 [tool.ruff]
 target-version = "py310"