Skip to content

Commit 33fc3f8

Browse files
gustavoliraclaude
andcommitted
refactor: dedupe coverage-tag rewrite + document E2E_NIGHTLY_COVERAGE
Review follow-ups on the nightly coverage swap: - Extract toCoverageImageRef() so the `:(tag)__coverage` rewrite (and the greedy-up-to-! regex) lives in one documented place instead of being duplicated across PR mode and nightly resolution. - Hoist the workspace-root resolution to a single workspaceRoot const (was computed twice). - Document the new behavior per the repo convention: a changelog entry, an E2E_NIGHTLY_COVERAGE row in the environment-variables reference, and a coverage-swap section in plugin-metadata-resolution. No behavior change; full suite still passes (yarn check + tests green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7a53838 commit 33fc3f8

4 files changed

Lines changed: 25 additions & 9 deletions

File tree

docs/changelog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file.
77
### Added
88

99
- **E2E coverage collection auto-fixture**: New `_coverageCollector` automatic fixture collects Istanbul coverage (`window.__coverage__`) from the browser after each test and writes per-test JSON files to `<outputDir>/coverage/`. Enabled via `E2E_COLLECT_COVERAGE=true`. Zero overhead when disabled — no `page.evaluate` call or fs operations. Designed for use with instrumented dynamic plugin builds (nyc instrument).
10+
- **Nightly coverage image swap** (`E2E_NIGHTLY_COVERAGE`): In nightly plugin resolution, an explicit opt-in swaps a rolled-out frontend plugin (workspace with a `coverage-anchors/` directory) from its released OCI image to the instrumented `__coverage` variant, so a coverage-dedicated nightly run can collect browser coverage. Gated separately from the ambient `E2E_COLLECT_COVERAGE` so the functional nightly's resolution is unchanged; `{{inherit}}` (DPDY) plugins are never swapped.
1011

1112
### Fixed
1213

docs/guide/configuration/environment-variables.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ These control automatic plugin configuration injection from metadata files.
4848
| ------------------------------------- | ------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
4949
| `GIT_PR_NUMBER` | PR number (set by OpenShift CI) | Enables OCI URL generation for PR builds |
5050
| `E2E_NIGHTLY_MODE` | When `"true"`, activates nightly mode | Plugins in `default.packages.yaml` with OCI metadata use `{{inherit}}` (RHDH resolves both OCI tag and config from DPDY); other OCI plugins use full metadata refs with config injection |
51+
| `E2E_NIGHTLY_COVERAGE` | When `"true"`, opt into the nightly coverage image swap | In nightly resolution, a rolled-out frontend plugin (workspace has `coverage-anchors/`) gets its released OCI tag swapped to the instrumented `__coverage` variant so the run can collect browser coverage. Off by default — the functional nightly is unaffected; `{{inherit}}` plugins are never swapped |
5152
| `RHDH_SKIP_PLUGIN_METADATA_INJECTION` | When `"true"`, disables metadata injection | Local-only opt-out (ignored when `CI=true`) |
5253
| `RELEASE_BRANCH_NAME` | Release branch (set by OpenShift CI step registry) | Used to fetch `default.packages.yaml` for DPDY resolution in nightly mode. Required in CI, defaults to `main` locally |
5354
| `NIGHTLY_DPDY_OCI_REGISTRY` | OCI registry for `{{inherit}}` refs | Overrides default `registry.access.redhat.com/rhdh` for all plugins using `{{inherit}}` in nightly mode |

docs/overlay/reference/plugin-metadata-resolution.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,20 @@ For each plugin, the resolver checks in order:
113113
114114
4. Use metadata's dynamicArtifact as-is
115115
(OCI ref → OCI ref, wrapper path → wrapper path)
116+
In a dedicated coverage run (E2E_NIGHTLY_COVERAGE=true), a frontend plugin
117+
whose workspace is rolled out (has a coverage-anchors/ directory) gets its
118+
OCI tag swapped to the instrumented __coverage variant:
119+
oci://ghcr.io/.../plugin:bs_X__Y!alias → ...:bs_X__Y__coverage!alias
116120
```
117121

118122
Metadata is the source of truth for the package reference, except for plugins in `default.packages.yaml` with OCI metadata in nightly mode — these use `{{inherit}}` so RHDH resolves both the OCI tag and config from its built-in DPDY, testing the exact versions and configuration shipped in the RC.
119123

124+
### Coverage image swap (`E2E_NIGHTLY_COVERAGE`)
125+
126+
A nightly run can only collect browser coverage if RHDH deploys the **instrumented** `__coverage` plugin image (built by the overlay release publish). When `E2E_NIGHTLY_COVERAGE=true`, step 4 above swaps a rolled-out frontend plugin's released OCI tag to its `__coverage` variant.
127+
128+
This is a separate flag from the ambient `E2E_COLLECT_COVERAGE` (which only toggles the collector fixture) on purpose: the functional nightly runs with `E2E_COLLECT_COVERAGE=true` by default, and the `__coverage` variant is built non-fatally, so swapping there could point at a tag that doesn't exist and break the deployment. The explicit `E2E_NIGHTLY_COVERAGE` opt-in keeps the functional nightly's resolution unchanged; only a coverage-dedicated run (which ensures the images exist) sets it. Plugins resolved via `{{inherit}}` are never swapped — those are RHDH's catalog images, which can't be instrumented.
129+
120130
## Resolution Scenarios
121131

122132
The tables below show what happens to each plugin type in PR check and nightly modes. Local dev behaves the same as PR check (metadata refs + full config injection).

src/utils/plugin-metadata.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,13 @@ function toDisplayName(packageName: string): string {
206206
return packageName.replace(/^@/, "").replace(/\//g, "-");
207207
}
208208

209+
// Append the __coverage suffix to an OCI image tag, before the optional
210+
// !<extractPath>. The instrumented variant the overlay builds. The regex is
211+
// greedy up to the first `!`, so the suffix always lands at the end of the tag.
212+
function toCoverageImageRef(ref: string): string {
213+
return ref.replace(/(:[^!]+)/, "$1__coverage");
214+
}
215+
209216
// ── Metadata Loading ──────────────────────────────────────────────────────────
210217

211218
export const DEFAULT_METADATA_PATH = "../metadata";
@@ -464,15 +471,16 @@ async function resolvePluginPackages(
464471
metadataPath: string,
465472
dpdyPackages: Set<string> | null = null,
466473
): Promise<PluginEntry[]> {
474+
const workspaceRoot = path.resolve(metadataPath, "..");
475+
467476
// Build PR OCI URLs if applicable
468477
const prNumber = process.env.GIT_PR_NUMBER;
469478
let prOciUrls: Map<string, string> | null = null;
470479
if (prNumber) {
471480
console.log(
472481
`[PluginMetadata] PR build detected (PR #${prNumber}), fetching OCI URLs...`,
473482
);
474-
const workspacePath = path.resolve(metadataPath, "..");
475-
prOciUrls = await getOCIUrlsForPR(workspacePath, prNumber);
483+
prOciUrls = await getOCIUrlsForPR(workspaceRoot, prNumber);
476484
}
477485

478486
// A dedicated coverage run swaps rolled-out frontend plugins to their
@@ -489,9 +497,7 @@ async function resolvePluginPackages(
489497
// the swap to rolled-out workspaces.
490498
const coverageSwap =
491499
process.env.E2E_NIGHTLY_COVERAGE === "true" &&
492-
fs.existsSync(
493-
path.join(path.resolve(metadataPath, ".."), "coverage-anchors"),
494-
);
500+
fs.existsSync(path.join(workspaceRoot, "coverage-anchors"));
495501

496502
return plugins.map((plugin) => {
497503
const pkg = plugin.package;
@@ -509,9 +515,7 @@ async function resolvePluginPackages(
509515
const usesCoverage =
510516
process.env.E2E_COLLECT_COVERAGE === "true" &&
511517
metadata.role === "frontend-plugin";
512-
const resolved = usesCoverage
513-
? prUrl.replace(/(:[^!]+)/, "$1__coverage")
514-
: prUrl;
518+
const resolved = usesCoverage ? toCoverageImageRef(prUrl) : prUrl;
515519
console.log(`[PluginMetadata] PR: ${pkg}${resolved}`);
516520
return { ...plugin, package: resolved };
517521
}
@@ -537,7 +541,7 @@ async function resolvePluginPackages(
537541
if (metadata.packagePath.startsWith("oci://")) {
538542
const resolved =
539543
coverageSwap && metadata.role === "frontend-plugin"
540-
? metadata.packagePath.replace(/(:[^!]+)/, "$1__coverage")
544+
? toCoverageImageRef(metadata.packagePath)
541545
: metadata.packagePath;
542546
console.log(`[PluginMetadata] ${pkg}${resolved}`);
543547
return { ...plugin, package: resolved };

0 commit comments

Comments
 (0)