Repository navigation
Run lifecycle, logging and persistency module tests for requirement coverage - #385
Merged
Merged
Conversation
…overage Execute three module-owned test suites in the Code Quality & Documentation job, before the documentation build, so the docs source code linker turns their PartiallyVerifies/FullyVerifies properties into testlink attributes on the respective requirement needs of the verification reports: - Lifecycle integration tests (@score_lifecycle//tests/integration/...): their test-framework deps (score_itf, the rules_oci extension) are exposed to this non-root consumer by //patches/lifecycle:002-undev-integration-test-deps.patch; docker mode is selected via the lifecycle config flag, qemu variants skip. - Logging component tests (@score_logging//score/test/component/...): deps (score_itf, rules_oci, score_rules_imagefs) exposed by //patches/logging:002-undev-component-test-deps.patch; docker image variant is the linux default, qnx variants skip. - Persistency component integration tests (@score_persistency//score/kvs/tests/test_cases:cit): score_py_pytest host targets carrying comp_req__kvs__* annotations, no patch needed. The docker-based suites use --flaky_test_attempts=3 to absorb a known race in the ITF docker plugin's PID read. A throwaway push-triggered preview workflow (delete before merge) exercises the new steps and verifies the lifecycle/logging/persistency requirements gain testlinks, since the real pull_request_target job reads its YAML from main.
antonkri
marked this pull request as ready for review
October 2, 2026 11:00
antonkri
requested review from
AlexanderLanin,
FScholPer,
MaximilianSoerenPollak,
PiotrKorkus,
dcalavrezo-qorix,
lurtz,
nradakovic,
opajonk and
pawelrutkaq
as code owners
October 2, 2026 11:00
|
The created documentation from the pull request is available at: docu-html |
anmittag
previously approved these changes
Oct 2, 2026
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The Bazel lockfile is stale, and the Logging patch breaks QNX test images.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Runs module-owned tests before documentation generation so requirement coverage links appear in verification reports.
Changes:
- Adds Lifecycle, Logging, and Persistency test execution to CI.
- Exposes transitive test dependencies through module patches.
- Regenerates Bazel module overrides from
known_good.json.
| File | Description |
|---|---|
.github/workflows/test_and_docs.yml |
Runs module tests before documentation generation. |
known_good.json |
Registers Lifecycle and Logging patches. |
bazel_common/score_modules_target_sw.MODULE.bazel |
Adds generated patch overrides. |
patches/lifecycle/002-undev-integration-test-deps.patch |
Exposes Lifecycle integration-test dependencies. |
patches/logging/002-undev-component-test-deps.patch |
Exposes Logging component-test dependencies. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "bazel_patches": [ | ||
| "//patches/lifecycle:001-migrate-baselibs-rust-targets.patch" | ||
| "//patches/lifecycle:001-migrate-baselibs-rust-targets.patch", | ||
| "//patches/lifecycle:002-undev-integration-test-deps.patch" |
Comment on lines
+33
to
+44
| diff --git a/score/test/component/defs.bzl b/score/test/component/defs.bzl | ||
| index 665f891a..0177a698 100644 | ||
| --- a/score/test/component/defs.bzl | ||
| +++ b/score/test/component/defs.bzl | ||
| @@ -66,7 +66,6 @@ def py_logging_itf_test(name, srcs, filesystem, filesystem_pkg, extra_oci_tars = | ||
| filesystem_pkg, | ||
| ], | ||
| build_file = "{}:init_build".format(_ENV), | ||
| - extra_build_files = ["{}:system_build".format(_ENV)], | ||
| target_compatible_with = [ | ||
| "@platforms//cpu:x86_64", | ||
| "@platforms//os:qnx", |
PiotrKorkus
requested changes
Oct 5, 2026
| bazel test --lockfile_mode=error --config=linux-x86_64 \ | ||
| --flaky_test_attempts=3 \ | ||
| --@score_lifecycle//config:integration_mode=docker \ | ||
| @score_lifecycle//tests/integration/... |
Contributor
There was a problem hiding this comment.
we need to handle it within quality_runners.py otherwise we will block ourselves during upgrades due to pull_request_target
Move the lifecycle, logging and persistency integration/component test invocations out of the workflow YAML and into scripts/quality_runners.py (new --integration-tests mode). The Code Quality & Documentation job runs via pull_request_target, which reads its YAML from the base branch. Hardcoding the module test targets and flags there means a module upgrade PR cannot adjust them in lockstep, which would block such upgrades. quality_runners.py is checked out from the PR head, so keeping the invocations there lets an upgrade PR change them.
Address review: integrate the module integration tests the same way as the unit tests instead of a hardcoded list in quality_runners.py. - Add optional `integration_test_targets` (module-relative labels) and `integration_test_config` (extra bazel flags, without the leading --) to the module Metadata model; both are emitted back only when set. - Declare them in known_good.json for score_lifecycle, score_logging and score_persistency (same targets/flags as before). - quality_runners.py now loops over the target_sw modules, exactly like the unit-test loop, and runs each module's integration suite when it declares one; the hardcoded IntegrationTest list is gone.
PiotrKorkus
reviewed
Oct 5, 2026
Comment on lines
+154
to
+160
| # Emitted only when set so the key stays absent for the vast majority of | ||
| # modules that have no integration tests wired into the integration. | ||
| if self.integration_test_targets: | ||
| result["integration_test_targets"] = self.integration_test_targets | ||
| if self.integration_test_config: | ||
| result["integration_test_config"] = self.integration_test_config | ||
| return result |
Contributor
There was a problem hiding this comment.
should be always returned, just use default values
| # source code linker turns the tests' Verifies properties into `testlink`s | ||
| # on the lifecycle, logging and persistency requirement needs. | ||
| run: | | ||
| python ./scripts/quality_runners.py --integration-tests |
Contributor
There was a problem hiding this comment.
shall we benefit from cache also here, same as for UTs?
Return integration_test_targets and integration_test_config from Metadata.to_dict() unconditionally with their default values, matching the sibling keys, instead of omitting them when empty.
Thread --trust-cache through run_integration_tests so it adds --nocache_test_results only for authoritative (tag) runs, mirroring the unit-test path. The workflow step now passes --trust-cache on every non-tag run so the shared disk cache stays populated.
PiotrKorkus
approved these changes
Oct 5, 2026
antonkri
added a commit
to antonkri/reference_integration
that referenced
this pull request
Oct 6, 2026
The module integration test step (added in eclipse-score#385) is red on main because two of the three configured modules fail for module-internal reasons: - score_persistency: cit_rust fails to build its python test venv with "Collision resolution not yet implemented! Saw nodejs_wheel/executable.py twice" (surfaced by the eclipse-score#383 persistency bump to 323950c, which also dropped several bazel patches). - score_logging: the datarouter component tests fail with "TimeoutError: Datarouter not ready within 10.0s". Remove the integration_test_targets/integration_test_config for these two modules so the pipeline is green again, keeping score_lifecycle (which passes). The persistency/logging integration tests can be re-enabled once the underlying venv collision and datarouter timeout are fixed.
PiotrKorkus
pushed a commit
that referenced
this pull request
Oct 6, 2026
* Add psutil to feature integration test requirements The feature integration tests add a lifecycle persistency recovery test (tests/lifecycle/test_lifecycle_persistency_recovery.py) that imports psutil, but psutil was not declared in the test requirements. The fit_*_lifecycle_persistency targets therefore fail during pytest collection with "ModuleNotFoundError: No module named 'psutil'". Add psutil to requirements.txt and the generated lock so it is part of all_requirements and available to the feature integration test venv. * Drop failing persistency and logging module integration tests The module integration test step (added in #385) is red on main because two of the three configured modules fail for module-internal reasons: - score_persistency: cit_rust fails to build its python test venv with "Collision resolution not yet implemented! Saw nodejs_wheel/executable.py twice" (surfaced by the #383 persistency bump to 323950c, which also dropped several bazel patches). - score_logging: the datarouter component tests fail with "TimeoutError: Datarouter not ready within 10.0s". Remove the integration_test_targets/integration_test_config for these two modules so the pipeline is green again, keeping score_lifecycle (which passes). The persistency/logging integration tests can be re-enabled once the underlying venv collision and datarouter timeout are fixed.
This was referenced Oct 6, 2026
antonkri
added a commit
that referenced
this pull request
Oct 6, 2026
The score_persistency KVS integration tests (//score/kvs/tests/test_cases:cit,
i.e. cit_cpp + cit_rust) could not build in reference_integration: building
their test venv failed with
× Collision resolution not yet implemented!
Saw nodejs_wheel/bin/corepack twice (then iniconfig, pytest, ...)
Root cause: the venv is assembled by score_tooling's score_virtualenv, which
does py_venv(deps = <@pip_tooling all_requirements> + reqs + ...). persistency
passes reqs = all_requirements of its own test hub (@pip_score_persistency_venv_test,
renamed by patch 003). That hub re-vendors score_tooling's
python_basics/requirements.txt (the test lock lists it as a compile src), so
basedpyright, iniconfig, nodejs-wheel-binaries, packaging, pluggy, pygments and
pytest exist in BOTH hubs. aspect_rules_py 1.5.2's venv tool cannot merge two
different repos that ship the same file (collision resolution is unimplemented
regardless of package_collisions=warning/ignore), so venv creation aborts.
This surfaced only after persistency was bumped to 323950c (#383) and the module
integration-test step was added (#385); it is the first time these tests ran in
reference_integration.
Fix (patches/persistency/006-dedupe-test-venv-pip-hub.patch): drop the seven
packages that score_tooling's @pip_tooling hub already provides from persistency's
test lock, so each package is contributed by exactly one hub. pytest and friends
come from @pip_tooling (added by score_virtualenv); persistency's hub keeps only
its own unique deps (psutil, pytest-env, pytest-html, pytest-metadata,
pytest-repeat, testing-utils, jinja2, markupsafe).
Verified locally (no docker): both
@score_persistency//score/kvs/tests/test_cases:python_tc_venv and
@score_persistency//score/kvs/tests/test_cases:cit build successfully with the
patch applied.
Re-enable the persistency integration_test_targets/integration_test_config in
known_good.json (removed in #386) so CI exercises the KVS integration tests.
antonkri
added a commit
that referenced
this pull request
Oct 7, 2026
…overrides (#388) * Enable persistency integration tests and fix test-venv pip hub collision The score_persistency KVS integration tests (//score/kvs/tests/test_cases:cit, i.e. cit_cpp + cit_rust) could not build in reference_integration: building their test venv failed with × Collision resolution not yet implemented! Saw nodejs_wheel/bin/corepack twice (then iniconfig, pytest, ...) Root cause: the venv is assembled by score_tooling's score_virtualenv, which does py_venv(deps = <@pip_tooling all_requirements> + reqs + ...). persistency passes reqs = all_requirements of its own test hub (@pip_score_persistency_venv_test, renamed by patch 003). That hub re-vendors score_tooling's python_basics/requirements.txt (the test lock lists it as a compile src), so basedpyright, iniconfig, nodejs-wheel-binaries, packaging, pluggy, pygments and pytest exist in BOTH hubs. aspect_rules_py 1.5.2's venv tool cannot merge two different repos that ship the same file (collision resolution is unimplemented regardless of package_collisions=warning/ignore), so venv creation aborts. This surfaced only after persistency was bumped to 323950c (#383) and the module integration-test step was added (#385); it is the first time these tests ran in reference_integration. Fix (patches/persistency/006-dedupe-test-venv-pip-hub.patch): drop the seven packages that score_tooling's @pip_tooling hub already provides from persistency's test lock, so each package is contributed by exactly one hub. pytest and friends come from @pip_tooling (added by score_virtualenv); persistency's hub keeps only its own unique deps (psutil, pytest-env, pytest-html, pytest-metadata, pytest-repeat, testing-utils, jinja2, markupsafe). Verified locally (no docker): both @score_persistency//score/kvs/tests/test_cases:python_tc_venv and @score_persistency//score/kvs/tests/test_cases:cit build successfully with the patch applied. Re-enable the persistency integration_test_targets/integration_test_config in known_good.json (removed in #386) so CI exercises the KVS integration tests. * Bump persistency to latest main and refresh integration-test overrides persistency fixed the test-venv pip hub collision upstream (#463 migrated to score_pytest from score_tools, #466 renamed the hub to pip_score_persistency_venv_test and dropped the unused python_tc_venv IDE venv). Bump the pinned commit to current main (961c0a0) so the reference_integration graph picks up those fixes directly. Consequences of the bump: - Drop 006-dedupe-test-venv-pip-hub.patch: the colliding python_tc_venv it worked around no longer exists upstream. - Regenerate 003/004/005 against the new MODULE.bazel (the old context no longer matched, so Bazel failed to apply them during fetch). - Add 006-tools / 007-sbom: the new main declares score_tools and score_sbom as dev_dependency, but reference_integration (root) needs them non-dev because the test BUILD loads @score_tools//score_pytest and the module root BUILD loads @score_sbom//:defs.bzl. Regenerated bazel_common/score_modules_target_sw.MODULE.bazel and refreshed MODULE.bazel.lock accordingly. * Pin score_tools to the $(locations) fix for persistency cit tests Persistency's integration test //score/kvs/tests/test_cases:cit failed in analysis: cit_cpp: label '...:cit_files' in $(location) expression expands to more than one file, please use $(locations ...) instead. score_pytest (from score_tools) only emits the correct $(locations ...) for multi-file srcs starting at a commit past the registry version 0.0.3. Persistency's own MODULE.bazel git_overrides score_tools to 4b48cec for exactly this fix, but Bzlmod ignores non-root overrides, so RI (root) was resolving score_tools 0.0.3 from the registry (still buggy). Add score_tools to known_good.json pinned to the same commit so the root module applies the override, regenerate the tooling MODULE fragment and refresh the lockfile. * Consolidate persistency dev_dependency flips into one patch The five patches 003-007 all edited the same file (MODULE.bazel) and four of them touched the same hunk region, so none of 005/006/007 applied standalone - they only worked as a sequential chain in exactly the known_good.json order, each carrying the previous ones' edits as context. Reordering or dropping any entry silently broke the rest. Replace them with a single patch containing two clean hunks. Same net effect: flip googletest, score_sbom, score_rules_imagefs, score_tools and the pip extension to non-dev, because their apparent names appear in BUILD/.bzl files that reference_integration evaluates (//:BUILD for the docs bundle, test_cases/BUILD for the pip hub). Extensions RI never evaluates, such as sbom_metadata and imagefs, stay dev on purpose. Verified: the patch applies standalone, and both @score_persistency//score/kvs/tests/test_cases:cit and @score_persistency//:docs_bundle resolve with --lockfile_mode=error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Executes three module-owned test suites in the Code Quality & Documentation job (before the documentation build) so the docs source code linker turns their
PartiallyVerifies/FullyVerifiesproperties intotestlinkattributes on the corresponding requirement needs of the verification reports.@score_lifecycle//tests/integration/...feat_req__lifecycle__*+ 14comp_req__launch_man__*patches/lifecycle/002-undev-integration-test-deps.patch@score_logging//score/test/component/...comp_req__log__*/comp_req__data_router__*patches/logging/002-undev-component-test-deps.patch@score_persistency//score/kvs/tests/test_cases:citcomp_req__kvs__*score_py_pytest)Details
score_itf, therules_ociextension, and for loggingscore_rules_imagefs) aredev_dependencyin the respective modules and therefore stripped for this non-root consumer. The two002patches un-dev exactly the pieces needed so the test packages load. Docker mode is the executed variant; the qemu/qnx variants aretarget_compatible_withincompatible and skip automatically.--flaky_test_attempts=3to absorb a known race in the ITF docker plugin's PID read (echo $$racing the launched binary's stdout).cit_cpp+cit_rust) are hostscore_py_pytesttargets that drive the KVS C++/Rust scenarios directly - no Docker, no patch. They are excluded from the standard persistency test run, so they are executed explicitly here.known_good.jsonregenerated viascripts/known_good/update_module_from_known_good.py;MODULE.bazel.lockunchanged.Throwaway preview
.github/workflows/zz-preview-module-coverage.ymlis a push-triggered throwaway workflow (the realpull_request_targetjob reads its YAML frommain, so new steps wouldn't run in the PR). It runs the three suites, dumps the recordedPartiallyVerifies, builds the docs and asserts every expected requirement gains atestlink. Delete this file before merging.