From 35fdbc5c63a97f48a00edb8304cd0257d185f909 Mon Sep 17 00:00:00 2001 From: Ben Copeland Date: Thu, 16 Jul 2026 16:09:25 +0100 Subject: [PATCH 1/5] ci: target the main branch instead of mainline The workflows referenced a mainline branch that does not exist in this repository; the default branch is main. Basic CI and CodeQL are gated on `branches: [ mainline ]`, so they never triggered for pull requests targeting main. The coverage check runs on all branches, but failed at `git checkout origin/mainline` before it could measure anything. Signed-off-by: Ben Copeland --- .github/workflows/basic-ci.yml | 4 ++-- .github/workflows/codeql.yml | 4 ++-- .github/workflows/coverage-check.yml | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/basic-ci.yml b/.github/workflows/basic-ci.yml index 0032d3b..0598b97 100644 --- a/.github/workflows/basic-ci.yml +++ b/.github/workflows/basic-ci.yml @@ -2,9 +2,9 @@ name: Basic CI on: push: - branches: [ mainline ] + branches: [ main ] pull_request: - branches: [ mainline ] + branches: [ main ] permissions: contents: read diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index d1fb096..e35a710 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -2,9 +2,9 @@ name: "CodeQL" on: push: - branches: [ mainline ] + branches: [ main ] pull_request: - branches: [ mainline ] + branches: [ main ] jobs: analyze: diff --git a/.github/workflows/coverage-check.yml b/.github/workflows/coverage-check.yml index a84ea13..87847c5 100644 --- a/.github/workflows/coverage-check.yml +++ b/.github/workflows/coverage-check.yml @@ -29,7 +29,7 @@ jobs: - name: Get base branch coverage run: | - git checkout origin/mainline + git checkout origin/main pip install -e ".[dev]" python -m pytest tests/ -m "not integration" --cov=src --cov-report=json:coverage-base.json --tb=no -q || true BASE_COVERAGE=$(python -c "import json; print(json.load(open('coverage-base.json'))['totals']['percent_covered'])") From 2f5cffea04698baf4a97a9fd8eedf58f8e2ae44f Mon Sep 17 00:00:00 2001 From: Ben Copeland Date: Thu, 16 Jul 2026 16:12:51 +0100 Subject: [PATCH 2/5] ci: drop Python 3.9 and 3.10 from the test matrix Signed-off-by: Ben Copeland --- .github/workflows/basic-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/basic-ci.yml b/.github/workflows/basic-ci.yml index 0598b97..11eea61 100644 --- a/.github/workflows/basic-ci.yml +++ b/.github/workflows/basic-ci.yml @@ -14,7 +14,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ["3.9", "3.10", "3.11", "3.12"] + python-version: ["3.11", "3.12"] steps: - uses: actions/checkout@v4 From 1ddc9a96013a980260daa9e936dfd79abef2373b Mon Sep 17 00:00:00 2001 From: Ben Copeland Date: Thu, 16 Jul 2026 16:24:54 +0100 Subject: [PATCH 3/5] lint: fix flake8 and pylint violations Basic CI has never run against this repository, so the tree has drifted out of line with .flake8 and .pylintrc. Fix the violations so the lint job can pass: F541 f-string without placeholders (cli.py) E501 line too long (cli.py) W391 blank line at end of file (artifacts.py) F401 unused imports (pull_labs_poller.py, test_pull_labs_translate.py) E306 missing blank line before nested definition (test_kcidb_submit.py) W0714 overlapping excepts, JSONDecodeError derives from ValueError (setup_validate.py) W0613 unused argument (test_kcidb_submit.py) W0212 protected access in tests (test_role_manager.py) These changes are extracted unmodified from PR #26 by Norbert Manthey, which fixes them alongside its feature work. Signed-off-by: Ben Copeland --- src/kernel_ci_cloud_labs/cli.py | 7 +++++-- src/kernel_ci_cloud_labs/core/artifacts.py | 2 -- src/kernel_ci_cloud_labs/pull_labs_poller.py | 3 +-- src/kernel_ci_cloud_labs/setup_validate.py | 4 ++-- tests/test_kcidb_submit.py | 6 ++++-- tests/test_pull_labs_translate.py | 1 - tests/test_role_manager.py | 1 + 7 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/kernel_ci_cloud_labs/cli.py b/src/kernel_ci_cloud_labs/cli.py index 2187bce..76df4ac 100644 --- a/src/kernel_ci_cloud_labs/cli.py +++ b/src/kernel_ci_cloud_labs/cli.py @@ -292,8 +292,11 @@ def main(): val_parser.add_argument("--bucket", help="S3 bucket to verify (and create with --fix)") val_parser.add_argument("--role", help="IAM role name used by VM instance profiles") val_parser.add_argument("--region", default="us-west-2", help="AWS region (default: us-west-2)") - val_parser.add_argument("--api-url", help=f"KernelCI API base URI (overrides $KERNELCI_API_BASE_URI)") - val_parser.add_argument("--fix", action="store_true", help="Create missing resources (S3 bucket) instead of just reporting them") + val_parser.add_argument("--api-url", help="KernelCI API base URI (overrides $KERNELCI_API_BASE_URI)") + val_parser.add_argument( + "--fix", action="store_true", + help="Create missing resources (S3 bucket) instead of just reporting them", + ) val_parser.set_defaults(func=cmd_setup_validate) args = parser.parse_args() diff --git a/src/kernel_ci_cloud_labs/core/artifacts.py b/src/kernel_ci_cloud_labs/core/artifacts.py index ebe330d..4b4de9b 100644 --- a/src/kernel_ci_cloud_labs/core/artifacts.py +++ b/src/kernel_ci_cloud_labs/core/artifacts.py @@ -262,5 +262,3 @@ def collect_run_artifacts( manifest_path, ) return manifest - - diff --git a/src/kernel_ci_cloud_labs/pull_labs_poller.py b/src/kernel_ci_cloud_labs/pull_labs_poller.py index 124e596..5057fe6 100644 --- a/src/kernel_ci_cloud_labs/pull_labs_poller.py +++ b/src/kernel_ci_cloud_labs/pull_labs_poller.py @@ -45,7 +45,6 @@ from kernel_ci_cloud_labs.kcidb_submit import ( build_test_row, - submit_tests, to_kcidb_status, ) from kernel_ci_cloud_labs.pull_labs_translate import translate_job @@ -314,7 +313,7 @@ def _validate_default_executor_deps() -> None: problems: List[str] = [] try: - import boto3 # noqa: F401,PLC0415 + import boto3 # noqa: F401,PLC0415 # pylint: disable=unused-import except ImportError as e: problems.append( f"boto3 import failed ({e}) — run: python3.11 -m pip install -e ." diff --git a/src/kernel_ci_cloud_labs/setup_validate.py b/src/kernel_ci_cloud_labs/setup_validate.py index bb51f5b..da0419b 100644 --- a/src/kernel_ci_cloud_labs/setup_validate.py +++ b/src/kernel_ci_cloud_labs/setup_validate.py @@ -200,7 +200,7 @@ def _check_bucket_policy_statement(s3, bucket_name: str, fix: bool) -> bool: print(f"✗ Could not read bucket policy ({code}): {e}") return False existing = None - except (ValueError, json.JSONDecodeError) as e: + except ValueError as e: print(f"✗ Bucket policy is not valid JSON: {e}") return False @@ -365,7 +365,7 @@ def check_kcidb_jwt() -> bool: # JWT payload is base64url; pad to a multiple of 4 before decoding. payload_b64 = parts[1] + "=" * (-len(parts[1]) % 4) payload = json.loads(base64.urlsafe_b64decode(payload_b64)) - except (ValueError, json.JSONDecodeError) as e: + except ValueError as e: print(f"✗ JWT payload not decodable: {e}") return False diff --git a/tests/test_kcidb_submit.py b/tests/test_kcidb_submit.py index 5c2a8d2..fd1dfeb 100644 --- a/tests/test_kcidb_submit.py +++ b/tests/test_kcidb_submit.py @@ -187,8 +187,10 @@ def test_submit_revision_posts_with_bearer_auth(self): class FakeResp: def __enter__(self): return self + def __exit__(self, *a): return False + def read(self): return b'{"status":"ok","id":"sub-1"}' @@ -237,7 +239,7 @@ def __enter__(self): return self def __exit__(self, *a): return False def read(self): return b'{}' - def fake_urlopen(req, timeout=None): + def fake_urlopen(req, timeout=None): # pylint: disable=unused-argument captured["body"] = json.loads(req.data.decode("utf-8")) return FakeResp() @@ -264,7 +266,7 @@ def __enter__(self): return self def __exit__(self, *a): return False def read(self): return b'{}' - def fake_urlopen(req, timeout=None): + def fake_urlopen(req, timeout=None): # pylint: disable=unused-argument captured["body"] = json.loads(req.data.decode("utf-8")) return FakeResp() diff --git a/tests/test_pull_labs_translate.py b/tests/test_pull_labs_translate.py index 68a33a2..b1a6298 100644 --- a/tests/test_pull_labs_translate.py +++ b/tests/test_pull_labs_translate.py @@ -9,7 +9,6 @@ from kernel_ci_cloud_labs.pull_labs_translate import ( DEFAULT_PLATFORM_MAP, - DEFAULT_TEST_TYPE_MAP, translate_job, ) diff --git a/tests/test_role_manager.py b/tests/test_role_manager.py index c3afbfb..dede843 100644 --- a/tests/test_role_manager.py +++ b/tests/test_role_manager.py @@ -1,4 +1,5 @@ """Unit tests for AWS Role Manager""" +# pylint: disable=protected-access __authors__ = ["Max Hubmann ", "Norbert Manthey "] __copyright__ = "Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved." From 5fbc7009dbb04bc511fa733d7fdc744b79cf359f Mon Sep 17 00:00:00 2001 From: Ben Copeland Date: Thu, 16 Jul 2026 16:25:06 +0100 Subject: [PATCH 4/5] setup: install the analysis dependencies with the dev extra pylint reports E0401 import-error for pandas, matplotlib and seaborn in the analysis package, because the dev extra does not install them and CI only installs ".[dev]". Add them to dev so lint can resolve the imports. Also set long_description_content_type, so the README renders as Markdown rather than plain text. Extracted unmodified from PR #26 by Norbert Manthey. Signed-off-by: Ben Copeland --- setup.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/setup.py b/setup.py index 8e39de9..520742b 100644 --- a/setup.py +++ b/setup.py @@ -25,6 +25,7 @@ version="1.0.0", description="Kernel-ci-cloud-labs", long_description=long_description, + long_description_content_type="text/markdown", package_dir={"": "src"}, packages=find_packages(where="src"), python_requires=">=3.11", @@ -45,6 +46,9 @@ "isort>=5.0", "pre-commit>=2.0", "pytest-cov>=2.0", + "pandas>=1.3.0", + "matplotlib>=3.4.0", + "seaborn>=0.11.0", ], "analysis": [ "pandas>=1.3.0", From 9168508f1c92c37e2445de51b809ff5537471b48 Mon Sep 17 00:00:00 2001 From: Ben Copeland Date: Thu, 16 Jul 2026 16:25:06 +0100 Subject: [PATCH 5/5] tests: update per-instance log URL test for maestro submission Commit 46d885e routed results through maestro and disabled direct kcidb submission, but left test_per_instance_rows_carry_log_url_and_stable_test_id asserting on the rows passed to submit_tests. Nothing calls submit_tests any more, so the test fails with KeyError: 'rows'. It went unnoticed because Basic CI never ran. Assert on the node outcome instead: log URLs now reach send_kcidb via artifacts.test_log and test_log_N, and the aggregated result is "fail" when any instance fails. The dropped assertions on stable test ids and misc.instance_id covered the disabled submit_tests path. Also fix the F401 and E127 violations in this file. Signed-off-by: Ben Copeland --- tests/test_pull_labs_poller.py | 57 +++++++++++++--------------------- 1 file changed, 22 insertions(+), 35 deletions(-) diff --git a/tests/test_pull_labs_poller.py b/tests/test_pull_labs_poller.py index 787c341..8b5e980 100644 --- a/tests/test_pull_labs_poller.py +++ b/tests/test_pull_labs_poller.py @@ -1,6 +1,7 @@ # SPDX-License-Identifier: Apache-2.0 # # Copyright (C) 2026 Collabora Limited +# pylint: disable=protected-access # Author: Denys Fedoryshchenko """Unit tests for pull_labs_poller (no network, no AWS).""" @@ -8,7 +9,6 @@ import json import logging import os -import tempfile import urllib.error from unittest.mock import patch @@ -626,14 +626,16 @@ class TestProcessEventNodeResult: def _run(self, poller, event, translate=None): translate = translate or {"return_value": {}} captured = {} - with patch.object(poller, "_claim_node", return_value=True), \ - patch.object( - poller, "_finish_node", - side_effect=lambda nid, outcome: captured.update(outcome=outcome), - ), \ - patch(_GET, return_value={"artifacts": {}}), \ - patch("kernel_ci_cloud_labs.pull_labs_poller.translate_job", **translate), \ - patch("kernel_ci_cloud_labs.pull_labs_poller.submit_tests", return_value={}): + with ( + patch.object(poller, "_claim_node", return_value=True), + patch.object( + poller, "_finish_node", + side_effect=lambda nid, outcome: captured.update(outcome=outcome), + ), + patch(_GET, return_value={"artifacts": {}}), + patch("kernel_ci_cloud_labs.pull_labs_poller.translate_job", + **translate), + ): poller.process_event(event) return captured["outcome"] @@ -682,9 +684,10 @@ def test_translate_failure_finishes_invalid_job_params(self): assert "missing artifacts.kernel" in outcome.error_msg def test_per_instance_rows_carry_log_url_and_stable_test_id(self): - """When executor returns per-instance rows with log_url, the submitted - KCIDB rows must each carry that URL and a test_id derived from the - instance_id (not the positional index).""" + """When executor returns per-instance rows with log_url, the node + outcome must carry those URLs in artifacts.test_log (first URL) and + test_log_N (subsequent URLs), and the node result must reflect the + aggregated per-instance statuses.""" per_test = [ {"name": "boot", "status": "PASS", "instance_id": "i-aaaa1111", "log_url": "https://b.s3.eu-west-1.amazonaws.com/a.log"}, @@ -695,31 +698,15 @@ def test_per_instance_rows_carry_log_url_and_stable_test_id(self): _minimal_kc(), job_executor=lambda cfg: (per_test, None), ) - seen = {} - with patch.object(p, "_claim_node", return_value=True), \ - patch.object(p, "_finish_node"), \ - patch(_GET, return_value={"artifacts": {}}), \ - patch("kernel_ci_cloud_labs.pull_labs_poller.translate_job", - return_value={}), \ - patch( - "kernel_ci_cloud_labs.pull_labs_poller.submit_tests", - side_effect=lambda url, jwt, origin, build_id, rows: seen.update(rows=rows), - ): - p.process_event(_job_event(node_id="ndX")) - - rows = seen["rows"] - assert len(rows) == 2 - by_id = {r["id"]: r for r in rows} - # test_id derived from instance_id => stable across retries. - assert set(by_id) == {"pullab_cloud_aws:ndX.i-aaaa1111", "pullab_cloud_aws:ndX.i-bbbb2222"} - # Per-row log_url survives the build_test_row pass-through. - assert by_id["pullab_cloud_aws:ndX.i-aaaa1111"]["log_url"] == \ + outcome = self._run(p, _job_event(node_id="ndX")) + + # One FAIL among results -> overall node result is "fail". + assert outcome.result == "fail" + # Log URLs are attached to the outcome artifacts for send_kcidb. + assert outcome.artifacts["test_log"] == \ "https://b.s3.eu-west-1.amazonaws.com/a.log" - assert by_id["pullab_cloud_aws:ndX.i-bbbb2222"]["log_url"] == \ + assert outcome.artifacts["test_log_1"] == \ "https://b.s3.eu-west-1.amazonaws.com/b.log" - # instance_id surfaces in misc for traceability. - assert by_id["pullab_cloud_aws:ndX.i-aaaa1111"]["misc"]["instance_id"] == "i-aaaa1111" - # Aggregated node outcome from per-instance statuses. # (one fail among two -> fail; verified indirectly via existing tests).