From 17f6fce6efb38dbb522007966c5e26485473f17a Mon Sep 17 00:00:00 2001 From: Peter Hedenskog Date: Mon, 20 Jul 2026 07:31:19 +0200 Subject: [PATCH] Test the visual metrics calculations in CI The Speed Index, visual progress and visual change math in visualmetrics-portable.py had no test coverage at all: the existing test file imported the mozilla-central Python packaging of the old script, which does not exist in this repo, so it could never run. Any change to the metric calculations would silently shift metrics for every user. The test data frames already in the repo now feed the shipped portable script directly, with the computed values pinned: First and Last Visual Change, Speed Index, the full visual progress curve, and the contentful/perceptual variants (skipped when OpenCV or pyssim is missing). A test for the upcoming frame-diff feature skips with a reason until that branch lands, so the two merge in either order. A new dedicated GitHub Action runs the suite; it finishes in under a second. Co-authored-by: Claude Fable 5 noreply@anthropic.com Change-Id: I625e90cf2b0edea9786ddf1d46bf7a69b240bc13 --- .github/workflows/visualmetrics.yml | 25 +++++ visualmetrics/test_visualmetrics.py | 143 ++++++++++++++++++++++------ 2 files changed, 141 insertions(+), 27 deletions(-) create mode 100644 .github/workflows/visualmetrics.yml diff --git a/.github/workflows/visualmetrics.yml b/.github/workflows/visualmetrics.yml new file mode 100644 index 0000000000..109dab477c --- /dev/null +++ b/.github/workflows/visualmetrics.yml @@ -0,0 +1,25 @@ +name: Visual metrics tests +on: + push: + branches: + - main + pull_request: + branches: + - main +jobs: + build: + runs-on: ubuntu-22.04 + steps: + - name: Harden Runner + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 + with: + egress-policy: audit + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + - name: Use Python 3.12 + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6 + with: + python-version: '3.12' + - name: Install visual metrics dependencies + run: python -m pip install --upgrade pip setuptools==70.0.0 pyssim OpenCV-Python Numpy + - name: Run visual metrics tests + run: python -m unittest discover -s visualmetrics -v diff --git a/visualmetrics/test_visualmetrics.py b/visualmetrics/test_visualmetrics.py index 97d791166a..ba07afd5a4 100644 --- a/visualmetrics/test_visualmetrics.py +++ b/visualmetrics/test_visualmetrics.py @@ -1,34 +1,123 @@ +"""Unit tests for visualmetrics-portable.py. + +The frames in test_data/ are real filmstrip frames from a browsertime +run. The expected values are what the portable script computes from +them, pinned so that any change to the metric math shows up as a +failing test instead of silently shifting metrics for every user. + +The script file name has a hyphen in it, so it is loaded via importlib +instead of a plain import. + +Run with: python -m unittest discover -s visualmetrics -v +""" + +import importlib.util +import logging +import shutil +import tempfile import unittest -import os +from pathlib import Path + +HERE = Path(__file__).resolve().parent -from browsertime.visualmetrics import ( - calculate_contentful_speed_index, - calculate_perceptual_speed_index, +_spec = importlib.util.spec_from_file_location( + "visualmetrics_portable", HERE / "visualmetrics-portable.py" ) +vm = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(vm) -HERE = os.path.dirname(__file__) +# The script logs a warning when no hero elements file is given; keep +# the test output clean. +logging.disable(logging.CRITICAL) + +EXPECTED_VISUAL_PROGRESS = ( + "0=0, 920=69, 1000=69, 1080=69, 1200=72, 1240=73, 1280=88, " + "1360=90, 1400=91, 1520=98, 2040=98, 2600=98, 3160=98, 3720=98, " + "4280=99, 4880=99, 5440=99, 6000=100" +) + + +def _importable(module): + try: + __import__(module) + return True + except ImportError: + return False class TestVisualMetrics(unittest.TestCase): - def setUp(self): - self.directory = "test_data" - images = os.listdir(os.path.join(HERE, self.directory)) - - def _p(image): - p = {} - p["time"] = int(image.split(".")[0].split("ms_")[-1]) - return p - - progress = [_p(image) for image in images if image.startswith("ms_")] - self.sorted_progress = sorted(progress, - key = lambda image: image['time']) - - def test_calculate_contentful_speed_index(self): - res = calculate_contentful_speed_index(self.sorted_progress, - self.directory) - self.assertEqual(res[0], 1188) - - def test_calculate_perceptual_speed_index(self): - res = calculate_perceptual_speed_index(self.sorted_progress, - self.directory) - self.assertEqual(res[0], 946) + @classmethod + def setUpClass(cls): + cls.tmp = tempfile.mkdtemp(prefix="vmtest-") + cls.histograms_file = str(Path(cls.tmp) / "histograms.json.gz") + vm.calculate_histograms(str(HERE / "test_data"), cls.histograms_file, True) + histograms = vm.load_histograms(cls.histograms_file, 0, 0) + cls.progress = vm.calculate_visual_progress(histograms) + + @classmethod + def tearDownClass(cls): + shutil.rmtree(cls.tmp, True) + + def _metrics(self): + metrics = vm.calculate_visual_metrics( + self.histograms_file, 0, 0, False, False, "test_data", None, None, {} + ) + return {metric["name"]: metric["value"] for metric in metrics} + + def test_first_and_last_visual_change(self): + metrics = self._metrics() + self.assertEqual(metrics["First Visual Change"], 920) + self.assertEqual(metrics["Last Visual Change"], 6000) + + def test_speed_index(self): + self.assertEqual(self._metrics()["Speed Index"], 1125) + + def test_visual_progress(self): + self.assertEqual(self._metrics()["Visual Progress"], EXPECTED_VISUAL_PROGRESS) + + def test_speed_index_math(self): + progress = [ + {"time": 0, "progress": 0}, + {"time": 1000, "progress": 50}, + {"time": 2000, "progress": 100}, + ] + self.assertEqual(vm.calculate_speed_index(progress), 1500) + + @unittest.skipUnless(_importable("cv2"), "OpenCV is not installed") + def test_contentful_speed_index(self): + value, _ = vm.calculate_contentful_speed_index(self.progress, "test_data") + self.assertEqual(value, 1079) + + @unittest.skipUnless(_importable("ssim"), "pyssim is not installed") + def test_perceptual_speed_index(self): + value, _ = vm.calculate_perceptual_speed_index(self.progress, "test_data") + self.assertEqual(value, 947) + + @unittest.skipUnless( + hasattr(vm, "create_frame_diffs"), + "create_frame_diffs is not in this branch yet", + ) + def test_create_frame_diffs(self): + frames_dir = Path(self.tmp) / "framediffs" + frames_dir.mkdir() + frames = sorted((HERE / "test_data").glob("ms_*.png")) + for frame in frames: + shutil.copy(frame, frames_dir) + + diffs = vm.create_frame_diffs(str(frames_dir)) + + self.assertEqual(len(diffs), len(frames) - 1) + expected_times = [int(frame.stem[3:]) for frame in frames[1:]] + self.assertEqual([diff["time"] for diff in diffs], expected_times) + for diff in diffs: + diff_file = frames_dir / "diff_{0:06d}.png".format(diff["time"]) + self.assertTrue(diff_file.is_file()) + self.assertGreaterEqual(diff["changedPixels"], 0) + self.assertGreaterEqual(diff["changedShare"], 0) + self.assertLessEqual(diff["changedShare"], 100) + # The first frame has no predecessor, so no diff for it. + self.assertFalse((frames_dir / "diff_000000.png").is_file()) + + +if __name__ == "__main__": + unittest.main()