test: add 'no-block-pr' pytest marker

The marker works identically to the 'nonci' marker, but with different
meaning: It is supposed to be used on tests that will run on every PR,
but whose failure should not block merging.

Additionally, switch from the --nonci option to using pytest's
built-in, marker-based test filtering. This has the advantage that
instead of skipping tests, we deselect them, reducing the log spam due
to skipped tests (which was particularly noticable in the performance
tests).

pytest.ini defines a default argument of `-m 'not nonci and not
no-block-pr'`, which deselects all nonci and optional tests. This can be
overwritten when invoking pytest (through devtool test) by setting -m,
for example to '-m optional' to only run optional tests, or to '-m
nonci' to only run nonci tests.

Signed-off-by: Patrick Roy <[email protected]>

Author: roypat

.buildkite/pipeline_cpu_template.py

@@ -24,7 +24,7 @@ class BkStep(str, Enum):
 cpu_template_test = {
     "rdmsr": {
         BkStep.COMMAND: [
-            "tools/devtool -y test -- -s -ra --nonci --log-cli-level=INFO integration_tests/functional/test_cpu_features.py -k 'test_cpu_rdmsr' "
+            "tools/devtool -y test -- -s -ra -m nonci --log-cli-level=INFO integration_tests/functional/test_cpu_features.py -k 'test_cpu_rdmsr' "
         ],
         BkStep.LABEL: "📖 rdmsr",
         "instances": ["m5d.metal", "m6a.metal", "m6i.metal"],
@@ -33,7 +33,7 @@ class BkStep(str, Enum):
     "cpuid_wrmsr": {
         "snapshot": {
             BkStep.COMMAND: [
-                "tools/devtool -y test -- -s -ra --nonci --log-cli-level=INFO integration_tests/functional/test_cpu_features.py -k 'test_cpu_wrmsr_snapshot or test_cpu_cpuid_snapshot'",
+                "tools/devtool -y test -- -s -ra -m nonci --log-cli-level=INFO integration_tests/functional/test_cpu_features.py -k 'test_cpu_wrmsr_snapshot or test_cpu_cpuid_snapshot'",
                 "mkdir -pv tests/snapshot_artifacts_upload/{instance}_{os}_{kv}",
                 "sudo mv tests/snapshot_artifacts/* tests/snapshot_artifacts_upload/{instance}_{os}_{kv}",
             ],
@@ -45,7 +45,7 @@ class BkStep(str, Enum):
             BkStep.COMMAND: [
                 "buildkite-agent artifact download tests/snapshot_artifacts_upload/{instance}_{os}_{kv}/**/* .",
                 "mv tests/snapshot_artifacts_upload/{instance}_{os}_{kv} tests/snapshot_artifacts",
-                "tools/devtool -y test -- -s -ra --nonci --log-cli-level=INFO integration_tests/functional/test_cpu_features.py -k 'test_cpu_wrmsr_restore or test_cpu_cpuid_restore'",
+                "tools/devtool -y test -- -s -ra -m nonci --log-cli-level=INFO integration_tests/functional/test_cpu_features.py -k 'test_cpu_wrmsr_restore or test_cpu_cpuid_restore'",
             ],
             BkStep.LABEL: "📸 load snapshot artifacts created on {instance} {snapshot_os} {snapshot_kv} to {restore_instance} {restore_os} {restore_kv}",
             BkStep.TIMEOUT: 30,
@@ -58,7 +58,7 @@ class BkStep(str, Enum):
     },
     "aarch64_cpu_templates": {
         BkStep.COMMAND: [
-            "tools/devtool -y test -- -s -ra --nonci --log-cli-level=INFO integration_tests/functional/test_cpu_features_aarch64.py"
+            "tools/devtool -y test -- -s -ra -m nonci --log-cli-level=INFO integration_tests/functional/test_cpu_features_aarch64.py"
         ],
         BkStep.LABEL: "📖 cpu templates",
         "instances": ["m6g.metal", "c7g.metal"],

.buildkite/pipeline_cross.py

@@ -26,7 +26,7 @@ def restore_step(label, src_instance, src_kv, dst_instance, dst_os, dst_kv):
         "command": [
             f"buildkite-agent artifact download snapshots/{src_instance}_{src_kv}/* .",
             f"mv -v snapshots/{src_instance}_{src_kv} snapshot_artifacts",
-            f"./tools/devtool -y test -- --nonci {k_val} integration_tests/functional/test_snapshot_restore_cross_kernel.py",
+            f"./tools/devtool -y test -- -m nonci {k_val} integration_tests/functional/test_snapshot_restore_cross_kernel.py",
         ],
         "label": label,
         "timeout": 30,

.buildkite/pipeline_perf.py

@@ -46,7 +46,7 @@ def build_group(test):
     test_path = test.pop("test_path")
     return group(
         label=test.pop("label"),
-        command=f"./tools/devtool -y test {devtool_opts} -- --nonci {test_path}",
+        command=f"./tools/devtool -y test {devtool_opts} -- -m nonci {test_path}",
         agent_tags=["ag=1"],
         artifacts=["./test_results/*"],
         instances=test.pop("instances"),

tests/conftest.py

@@ -92,7 +92,6 @@ def test_with_any_microvm(test_microvm_any):
 
 import pytest
 
-import framework.utils_cpuid as cpuid_utils
 import host_tools.cargo_build as build_tools
 from framework import defs, utils
 from framework.artifacts import ArtifactCollection, DiskArtifact, FirecrackerArtifact
@@ -126,32 +125,6 @@ def test_with_any_microvm(test_microvm_any):
 METRICS = get_metrics_logger()
 
 
-def pytest_configure(config):
-    """Pytest hook - initialization"""
-    config.addinivalue_line("markers", "nonci: mark test as nonci.")
-
-
-def pytest_addoption(parser):
-    """Pytest hook. Add command line options."""
-    parser.addoption("--nonci", action="store_true", help="run tests marked with nonci")
-
-
-def pytest_collection_modifyitems(config, items):
-    """Pytest hook. Skip some tests."""
-    skip_markers = {}
-
-    for skip_marker_name in ["nonci"]:
-        if not config.getoption(f"--{skip_marker_name}"):
-            skip_markers[skip_marker_name] = pytest.mark.skip(
-                reason=f"Skipping {skip_marker_name} test"
-            )
-
-    for item in items:
-        for skip_marker_name, skip_marker in skip_markers.items():
-            if skip_marker_name in item.keywords:
-                item.add_marker(skip_marker)
-
-
 @pytest.fixture(scope="function", autouse=True)
 def record_props(request, record_property):
     """Decorate test results with additional properties.
@@ -511,23 +484,13 @@ def rootfs_msrtools(request, record_property):
 def cpu_template(request, record_property):
     """Return all CPU templates supported by the vendor."""
     record_property("cpu_template", request.param)
-    if (
-        cpuid_utils.get_cpu_vendor() == cpuid_utils.CpuVendor.ARM
-        and not request.config.getoption("--nonci")
-    ):
-        pytest.skip("temporary skip on usual CI")
     return request.param
 
 
 @pytest.fixture(params=SUPPORTED_CUSTOM_CPU_TEMPLATES)
 def custom_cpu_template(request, record_property):
     """Return all dummy custom CPU templates supported by the vendor."""
     record_property("custom_cpu_template", request.param)
-    if (
-        cpuid_utils.get_cpu_vendor() == cpuid_utils.CpuVendor.ARM
-        and not request.config.getoption("--nonci")
-    ):
-        pytest.skip("temporary skip on usual CI")
     return request.param
 
 

tests/framework/utils_cpu_templates.py

@@ -6,6 +6,8 @@
 import json
 from pathlib import Path
 
+import pytest
+
 import framework.utils_cpuid as cpuid_utils
 
 # All existing CPU templates available on Intel
@@ -77,3 +79,10 @@ def name_list_to_tmpl_list(name_list):
 
 
 SUPPORTED_CUSTOM_CPU_TEMPLATES = get_supported_custom_cpu_templates()
+
+
+def nonci_on_arm(func):
+    """Temporary decorator used to mark specific cpu template related tests as nonci on ARM platforms"""
+    if cpuid_utils.get_cpu_vendor() == cpuid_utils.CpuVendor.ARM:
+        return pytest.mark.nonci(func)
+    return func

tests/integration_tests/functional/test_api.py

@@ -18,6 +18,7 @@
 from framework.artifacts import NetIfaceConfig, SnapshotType
 from framework.builder import MicrovmBuilder, SnapshotBuilder
 from framework.utils import get_firecracker_version_from_toml, is_io_uring_supported
+from framework.utils_cpu_templates import nonci_on_arm
 
 MEM_LIMIT = 1000000000
 
@@ -474,6 +475,7 @@ def test_api_machine_config(test_microvm_with_api):
     assert json["machine-config"]["smt"] is False
 
 
+@nonci_on_arm
 def test_api_cpu_config(test_microvm_with_api, custom_cpu_template):
     """
     Test /cpu-config PUT scenarios.

tests/integration_tests/functional/test_cpu_features_aarch64.py

@@ -9,6 +9,7 @@
 import pytest
 
 import framework.utils_cpuid as cpuid_utils
+from framework.utils_cpu_templates import nonci_on_arm
 
 PLATFORM = platform.machine()
 
@@ -95,6 +96,7 @@ def test_default_cpu_features(test_microvm_with_api, network_config):
     PLATFORM != "aarch64",
     reason="This is aarch64 specific test.",
 )
+@nonci_on_arm
 def test_cpu_features_with_static_template(
     test_microvm_with_api, network_config, cpu_template
 ):
@@ -113,6 +115,7 @@ def test_cpu_features_with_static_template(
     PLATFORM != "aarch64",
     reason="This is aarch64 specific test.",
 )
+@nonci_on_arm
 def test_cpu_features_with_custom_template(
     test_microvm_with_api, network_config, custom_cpu_template
 ):

tests/integration_tests/functional/test_cpu_template_helper.py

@@ -394,6 +394,7 @@ def test_host_fingerprint_change(test_microvm_with_api, cpu_template_helper, tmp
     )
 
 
+@nonci_on_arm
 def test_json_static_templates(
     test_microvm_with_api, cpu_template_helper, tmp_path, custom_cpu_template
 ):

tests/integration_tests/security/test_vulnerabilities.py

@@ -14,6 +14,7 @@
 from framework import utils
 from framework.artifacts import DEFAULT_NETMASK
 from framework.properties import global_props
+from framework.utils_cpu_templates import nonci_on_arm
 
 CHECKER_URL = "https://meltdown.ovh"
 CHECKER_FILENAME = "spectre-meltdown-checker.sh"
@@ -154,6 +155,7 @@ def test_spectre_meltdown_checker_on_restored_guest(
     global_props.instance == "c7g.metal" and global_props.host_linux_version == "4.14",
     reason="c7g host 4.14 requires modifications to the 5.10 guest kernel to boot successfully.",
 )
+@nonci_on_arm
 def test_spectre_meltdown_checker_on_guest_with_template(
     spectre_meltdown_checker,
     test_microvm_with_spectre_meltdown,
@@ -177,6 +179,7 @@ def test_spectre_meltdown_checker_on_guest_with_template(
     global_props.instance == "c7g.metal" and global_props.host_linux_version == "4.14",
     reason="c7g host 4.14 requires modifications to the 5.10 guest kernel to boot successfully.",
 )
+@nonci_on_arm
 def test_spectre_meltdown_checker_on_guest_with_custom_template(
     spectre_meltdown_checker,
     test_microvm_with_spectre_meltdown,
@@ -202,6 +205,7 @@ def test_spectre_meltdown_checker_on_guest_with_custom_template(
     global_props.instance == "c7g.metal" and global_props.host_linux_version == "4.14",
     reason="c7g host 4.14 requires modifications to the 5.10 guest kernel to boot successfully.",
 )
+@nonci_on_arm
 def test_spectre_meltdown_checker_on_restored_guest_with_template(
     spectre_meltdown_checker,
     test_microvm_with_spectre_meltdown,
@@ -228,6 +232,7 @@ def test_spectre_meltdown_checker_on_restored_guest_with_template(
     global_props.instance == "c7g.metal" and global_props.host_linux_version == "4.14",
     reason="c7g host 4.14 requires modifications to the 5.10 guest kernel to boot successfully.",
 )
+@nonci_on_arm
 def test_spectre_meltdown_checker_on_restored_guest_with_custom_template(
     spectre_meltdown_checker,
     test_microvm_with_spectre_meltdown,
@@ -292,6 +297,7 @@ def test_vulnerabilities_files_on_restored_guest(
     check_vulnerabilities_files_on_guest(dst_vm)
 
 
+@nonci_on_arm
 def test_vulnerabilities_files_on_guest_with_template(
     test_microvm_with_api,
     network_config,
@@ -305,6 +311,7 @@ def test_vulnerabilities_files_on_guest_with_template(
     check_vulnerabilities_files_on_guest(microvm)
 
 
+@nonci_on_arm
 def test_vulnerabilities_files_on_guest_with_custom_template(
     test_microvm_with_api,
     network_config,
@@ -322,6 +329,7 @@ def test_vulnerabilities_files_on_guest_with_custom_template(
     check_vulnerabilities_files_on_guest(microvm)
 
 
+@nonci_on_arm
 def test_vulnerabilities_files_on_restored_guest_with_template(
     test_microvm_with_api,
     network_config,
@@ -340,6 +348,7 @@ def test_vulnerabilities_files_on_restored_guest_with_template(
     check_vulnerabilities_files_on_guest(dst_vm)
 
 
+@nonci_on_arm
 def test_vulnerabilities_files_on_restored_guest_with_custom_template(
     test_microvm_with_api,
     network_config,

tests/pytest.ini

@@ -4,6 +4,11 @@ addopts =
     --tb=short
     --durations=10
     --showlocals
+    -m 'not nonci and not no_block_pr'
+
+markers =
+    no_block_pr: tests whose failure does not block PR merging.
+    nonci: mark test as nonci.
 
 ; Overwrite the default norecursedirs, which includes 'build'.
 norecursedirs = .*