chore(test): use record_property instead of returning from tests

Returning from a test is deprecated in pytest

```
build/test_binary_size.py::test_firecracker_binary_size
PytestReturnNotNoneWarning: Expected None, but returned
('2505192 B', '2520632 +/- 5.0% B'), which will be an error in a future
version of pytest.  Did you mean to use `assert` instead of `return`?
```

This was used by the test reporting framework to augment test results.
We will switch to a more mainstream way of doing test reports in a
subsequent commit.

Signed-off-by: Pablo Barbáchano <[email protected]>

Author: pb8o

tests/integration_tests/build/test_binary_size.py

@@ -8,6 +8,7 @@
 
 import host_tools.cargo_build as host
 
+
 MACHINE = platform.machine()
 """ Platform definition used to select the correct size target"""
 
@@ -33,7 +34,7 @@
 
 
 @pytest.mark.timeout(500)
-def test_firecracker_binary_size():
+def test_firecracker_binary_size(record_property):
     """
     Test if the size of the firecracker binary is within expected ranges.
 
@@ -48,14 +49,14 @@ def test_firecracker_binary_size():
         BINARY_SIZE_TOLERANCE,
     )
 
-    return (
-        f"{result} B",
-        f"{FC_BINARY_SIZE_TARGET} +/- {BINARY_SIZE_TOLERANCE * 100}% B",
+    record_property(
+        "firecracker_binary_size",
+        f"{result}B ({FC_BINARY_SIZE_TARGET}B ±{BINARY_SIZE_TOLERANCE:.0%})",
     )
 
 
 @pytest.mark.timeout(500)
-def test_jailer_binary_size():
+def test_jailer_binary_size(record_property):
     """
     Test if the size of the jailer binary is within expected ranges.
 
@@ -70,9 +71,9 @@ def test_jailer_binary_size():
         BINARY_SIZE_TOLERANCE,
     )
 
-    return (
-        f"{result} B",
-        f"{JAILER_BINARY_SIZE_TARGET} +/- {BINARY_SIZE_TOLERANCE * 100}% B",
+    record_property(
+        "jailer_binary_size",
+        f"{result}B ({JAILER_BINARY_SIZE_TARGET}B ±{BINARY_SIZE_TOLERANCE:.0%})",
     )
 
 

tests/integration_tests/build/test_coverage.py

@@ -46,7 +46,7 @@
 
 
 @pytest.mark.timeout(400)
-def test_coverage(test_fc_session_root_path, test_session_tmp_path):
+def test_coverage(test_fc_session_root_path, test_session_tmp_path, record_property):
     """Test line coverage for rust tests is within bounds.
 
     The result is extracted from the $KCOV_COVERAGE_FILE file created by kcov
@@ -131,6 +131,7 @@ def test_coverage(test_fc_session_root_path, test_session_tmp_path):
         )
     )
 
+    record_property(
+        "coverage", f"{coverage}% {coverage_target_pct}% ±{COVERAGE_MAX_DELTA:.2}%"
+    )
     assert coverage <= coverage_target_pct + COVERAGE_MAX_DELTA, coverage_high_msg
-
-    return (f"{coverage}%", f"{coverage_target_pct}% +/- {COVERAGE_MAX_DELTA * 100}%")

tests/integration_tests/performance/test_boottime.py

@@ -30,7 +30,7 @@ def test_no_boottime(test_microvm_with_api):
     assert not timestamps
 
 
-def test_boottime_no_network(test_microvm_with_api):
+def test_boottime_no_network(test_microvm_with_api, record_property):
     """
     Check boot time of microVM without a network device.
 
@@ -40,12 +40,11 @@ def test_boottime_no_network(test_microvm_with_api):
     vm.jailer.extra_args.update({"boot-timer": None})
     _ = _configure_and_run_vm(vm)
     boottime_us = _test_microvm_boottime(vm)
-    print("Boot time with no network is: " + str(boottime_us) + " us")
+    print(f"Boot time with no network is: {boottime_us} us")
+    record_property("boottime_no_network", f"{boottime_us} us < {MAX_BOOT_TIME_US} us")
 
-    return f"{boottime_us} us", f"< {MAX_BOOT_TIME_US} us"
 
-
-def test_boottime_with_network(test_microvm_with_api, network_config):
+def test_boottime_with_network(test_microvm_with_api, network_config, record_property):
     """
     Check boot time of microVM with a network device.
 
@@ -55,12 +54,13 @@ def test_boottime_with_network(test_microvm_with_api, network_config):
     vm.jailer.extra_args.update({"boot-timer": None})
     _tap = _configure_and_run_vm(vm, {"config": network_config, "iface_id": "1"})
     boottime_us = _test_microvm_boottime(vm)
-    print("Boot time with network configured is: " + str(boottime_us) + " us")
-
-    return f"{boottime_us} us", f"< {MAX_BOOT_TIME_US} us"
+    print(f"Boot time with network configured is: {boottime_us} us")
+    record_property(
+        "boottime_with_network", f"{boottime_us} us < {MAX_BOOT_TIME_US} us"
+    )
 
 
-def test_initrd_boottime(test_microvm_with_initrd):
+def test_initrd_boottime(test_microvm_with_initrd, record_property):
     """
     Check boot time of microVM when using an initrd.
 
@@ -70,9 +70,8 @@ def test_initrd_boottime(test_microvm_with_initrd):
     vm.jailer.extra_args.update({"boot-timer": None})
     _tap = _configure_and_run_vm(vm, initrd=True)
     boottime_us = _test_microvm_boottime(vm, max_time_us=INITRD_BOOT_TIME_US)
-    print("Boot time with initrd is: " + str(boottime_us) + " us")
-
-    return f"{boottime_us} us", f"< {INITRD_BOOT_TIME_US} us"
+    print(f"Boot time with initrd is: {boottime_us} us")
+    record_property("boottime_initrd", f"{boottime_us} us < {MAX_BOOT_TIME_US} us")
 
 
 def _test_microvm_boottime(vm, max_time_us=MAX_BOOT_TIME_US):

tests/integration_tests/performance/test_process_startup_time.py

@@ -15,7 +15,7 @@
 # TODO: Keep a `current` startup time in S3 and validate we don't regress
 
 
-def test_startup_time_new_pid_ns(test_microvm_with_api):
+def test_startup_time_new_pid_ns(test_microvm_with_api, record_property):
     """
     Check startup time when jailer is spawned in a new PID namespace.
 
@@ -24,20 +24,20 @@ def test_startup_time_new_pid_ns(test_microvm_with_api):
     microvm = test_microvm_with_api
     microvm.bin_cloner_path = None
     microvm.jailer.new_pid_ns = True
-    return _test_startup_time(microvm)
+    record_property("startup_time_new_pid_μs", _test_startup_time(microvm))
 
 
-def test_startup_time_daemonize(test_microvm_with_api):
+def test_startup_time_daemonize(test_microvm_with_api, record_property):
     """
     Check startup time when jailer spawns Firecracker in a new PID ns.
 
     @type: performance
     """
     microvm = test_microvm_with_api
-    return _test_startup_time(microvm)
+    record_property("startup_time_daemonize_μs", _test_startup_time(microvm))
 
 
-def test_startup_time_custom_seccomp(test_microvm_with_api):
+def test_startup_time_custom_seccomp(test_microvm_with_api, record_property):
     """
     Check the startup time when using custom seccomp filters.
 
@@ -46,8 +46,7 @@ def test_startup_time_custom_seccomp(test_microvm_with_api):
     microvm = test_microvm_with_api
 
     _custom_filter_setup(microvm)
-
-    return _test_startup_time(microvm)
+    record_property("startup_time_custom_seccomp_μs", _test_startup_time(microvm))
 
 
 def _test_startup_time(microvm):

tests/integration_tests/performance/test_versioned_serialization_benchmark.py

@@ -7,6 +7,9 @@
 import json
 import shutil
 import platform
+from pathlib import Path
+
+import pytest
 
 from framework import utils
 from framework.defs import FC_WORKSPACE_DIR
@@ -68,14 +71,15 @@ def _check_statistics(directory, mean):
         attribute = "no-crc"
 
     measure = BASELINES[proc_model[0]][bench][attribute]
-    low = round(measure["target"] - measure["delta"], 3)
-    high = round(measure["target"] + measure["delta"], 3)
-    assert low <= mean <= high, "Benchmark result {} has changed!".format(directory)
+    target, delta = measure["target"], measure["delta"]
+    assert target == pytest.approx(
+        mean, abs=delta, rel=1e-6
+    ), f"Benchmark result {directory} has changed!"
 
-    return directory, f"{mean} ms", f"{low} <= result <= {high}"
+    return f"{target - delta} <= result <= {target + delta}"
 
 
-def test_serialization_benchmark():
+def test_serialization_benchmark(monkeypatch, record_property):
     """
     Benchmark test for MicrovmState serialization/deserialization.
 
@@ -84,18 +88,16 @@ def test_serialization_benchmark():
     logger = logging.getLogger("serialization_benchmark")
 
     # Move into the benchmark directory
-    os.chdir(BENCHMARK_DIRECTORY)
+    monkeypatch.chdir(BENCHMARK_DIRECTORY)
 
     # Run benchmark test
     cmd = "cargo bench --target {}".format(DEFAULT_BUILD_TARGET)
     result = utils.run_cmd_sync(cmd)
     assert result.returncode == 0
 
-    results_and_criteria = ["", ""]
-
     # Parse each Criterion benchmark from the result folder and
     # check the results against a baseline
-    results_dir = os.path.join(FC_WORKSPACE_DIR, "build/vmm_benchmark")
+    results_dir = Path(FC_WORKSPACE_DIR) / "build/vmm_benchmark"
     for directory in os.listdir(results_dir):
         # Ignore the 'report' directory as it is of no use to us
         if directory == "report":
@@ -104,23 +106,16 @@ def test_serialization_benchmark():
         logger.info("Benchmark: %s", directory)
 
         # Retrieve the 'estimates.json' file content
-        json_file = os.path.join(
-            results_dir, "{}/{}".format(directory, "base/estimates.json")
-        )
-        with open(json_file, "r", encoding="utf-8") as read_file:
-            estimates = json.load(read_file)
+        json_file = results_dir / directory / "base/estimates.json"
+        estimates = json.loads(json_file.read_text())
 
         # Save the Mean measurement(nanoseconds) and transform it(milliseconds)
         mean = estimates["mean"]["point_estimate"] / NSEC_IN_MSEC
         logger.info("Mean: %f", mean)
 
-        res = _check_statistics(directory, round(mean, 3))
-
-        results_and_criteria[0] += f"{res[0]}: {res[1]}, "
-        results_and_criteria[1] += f"{res[0]}: {res[2]}, "
+        criteria = _check_statistics(directory, mean)
+        record_property(f"{directory}_ms", mean)
+        record_property(f"{directory}_criteria", criteria)
 
     # Cleanup the Target directory
     shutil.rmtree(results_dir)
-
-    # Return pretty formatted data for the test report.
-    return results_and_criteria[0], results_and_criteria[1]