From e7d7e2206cfd014fb470a2674188f29bbfb1885e Mon Sep 17 00:00:00 2001 From: quality Date: Sun, 11 Oct 2026 21:09:29 -0400 Subject: [PATCH] test: cover DirectoryFreshness's plain-text source fallbacks end-to-end DirectoryFreshness names each provenance source as plain text when that source carries no URL (lines 30 and 44), and neither arm had ever run in a browser: the committed data always carries both URLs, and the e2e coverage artifact of run 38182984387 reported the file at 69.23% regions with 19, 30, 44 and 49 uncovered. Both arms need their sentence to render while the URL is absent, which conflicts with the ordinary coverage build (where the anchors at lines 26 and 40 are measured from) and with the variant build (which drops both sentences). tests/e2e/fixtures/data-no-revision/ already holds that shape and ReferenceArchitectures returns at its revision guard before reading sources.architectures.repository, so the two URLs are cleared there rather than in a new build directory, which would add a full Docusaurus compile to the End-to-end coverage job. Closes #1328 Signed-off-by: quality --- tests/e2e-coverage-describe-gate.test.mjs | 1 + tests/e2e-data-fixture-validity.test.mjs | 11 +- .../fixtures/data-no-revision/members.json | 6 + .../fixtures/data-no-revision/metrics.json | 5 +- ...ember-directory-freshness-unlinked.spec.js | 147 ++++++++++++++++++ 5 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 tests/e2e/fixtures/data-no-revision/members.json create mode 100644 tests/e2e/member-directory-freshness-unlinked.spec.js diff --git a/tests/e2e-coverage-describe-gate.test.mjs b/tests/e2e-coverage-describe-gate.test.mjs index 3abfc7d5..871779fc 100644 --- a/tests/e2e-coverage-describe-gate.test.mjs +++ b/tests/e2e-coverage-describe-gate.test.mjs @@ -54,6 +54,7 @@ const COVERAGE_GATED = [ 'data-fixtures.spec.js', 'data-variants.spec.js', 'group-link-status-variant.spec.js', + 'member-directory-freshness-unlinked.spec.js', 'member-directory-freshness-variant.spec.js', 'metrics-empty-collections-variant.spec.js', 'metrics-sparkline.spec.js', diff --git a/tests/e2e-data-fixture-validity.test.mjs b/tests/e2e-data-fixture-validity.test.mjs index e348f182..9d4ef1e7 100644 --- a/tests/e2e-data-fixture-validity.test.mjs +++ b/tests/e2e-data-fixture-validity.test.mjs @@ -138,10 +138,19 @@ const EXPECTED = new Map([ adds: [], }, ], + [ + 'data-no-revision/members.json', + { + why: "clears sources.landscape.sourceUrl to reach DirectoryFreshness's plain-text 'cncf/landscape' arm; validate-members.mjs holds that field to the pinned landscape snapshot", + adds: [ + 'members.json.sources.landscape: landscape provenance must match enduser-landscape.json', + ], + }, + ], [ 'data-no-revision/metrics.json', { - why: 'clears sources.architectures.revision to reach the absent-revision guard in ReferenceArchitectures; the gate requires that revision', + why: "clears sources.architectures.revision to reach the absent-revision guard in ReferenceArchitectures, and sources.architectures.repository to reach DirectoryFreshness's plain-text 'cncf/architecture' arm; the gate requires that revision", adds: ['metrics.json: source revisions are required'], }, ], diff --git a/tests/e2e/fixtures/data-no-revision/members.json b/tests/e2e/fixtures/data-no-revision/members.json new file mode 100644 index 00000000..60c7e276 --- /dev/null +++ b/tests/e2e/fixtures/data-no-revision/members.json @@ -0,0 +1,6 @@ +{ + "description": "Clears sources.landscape.sourceUrl so src/components/MemberDirectory/DirectoryFreshness.js renders the plain-text 'cncf/landscape' arm of its link ternary at line 30 instead of the anchor at line 26. The arm needs the membership sentence to render -- sources.landscape.collectedAt parseable -- while the snapshot URL is absent, which is what a collector run that pins a snapshot it cannot address by URL leaves behind. It cannot go in the ordinary coverage build, whose anchor is the one tests/e2e/member-directory-freshness.spec.js asserts, nor in the variant build, which makes collectedAt unparseable and drops the sentence the arm lives in. It shares this build with the metrics overlay beside it because nothing else reads sources.landscape.sourceUrl and this build already renders the note, so it costs no additional compile.", + "set": { + "sources.landscape.sourceUrl": null + } +} diff --git a/tests/e2e/fixtures/data-no-revision/metrics.json b/tests/e2e/fixtures/data-no-revision/metrics.json index 4b66c8f7..79153eac 100644 --- a/tests/e2e/fixtures/data-no-revision/metrics.json +++ b/tests/e2e/fixtures/data-no-revision/metrics.json @@ -1,6 +1,7 @@ { - "description": "Clears sources.architectures.revision so src/components/ReferenceArchitectures/index.js takes the absent-revision guard at line 13 -- SyncStatus returns null and the whole provenance line goes with it. The field is a scalar on a singleton object of the one file the one /architectures page reads, so clearing it in the ordinary coverage build would not add a case: it would take lines 14-34 with it, including the `on ${syncDate}` arm the real build is the only one to reach. It cannot share the variant build either, which clears metrics.generatedAt to reach the ': null' syncDate arm at line 23 -- that arm is inside the guard this overlay closes, so one build cannot hold both shapes. That standoff is why the fixture mechanism takes a directory per build rather than a fixed second one. metrics.generatedAt is deliberately left present here: with it cleared this build would cover nothing the variant does not.", + "description": "Clears sources.architectures.revision so src/components/ReferenceArchitectures/index.js takes the absent-revision guard at line 13 -- SyncStatus returns null and the whole provenance line goes with it. The field is a scalar on a singleton object of the one file the one /architectures page reads, so clearing it in the ordinary coverage build would not add a case: it would take lines 14-34 with it, including the `on ${syncDate}` arm the real build is the only one to reach. It cannot share the variant build either, which clears metrics.generatedAt to reach the ': null' syncDate arm at line 23 -- that arm is inside the guard this overlay closes, so one build cannot hold both shapes. That standoff is why the fixture mechanism takes a directory per build rather than a fixed second one. metrics.generatedAt is deliberately left present here: with it cleared this build would cover nothing the variant does not. The same overlay clears sources.architectures.repository, which is what makes DirectoryFreshness render the plain-text 'cncf/architecture' arm of its link ternary (src/components/MemberDirectory/DirectoryFreshness.js line 44) instead of the anchor at line 40. That arm needs the architectures sentence to render -- metrics.generatedAt present -- while the repository URL is absent, so it cannot go in the variant build, which clears generatedAt and drops the sentence entirely, nor in the ordinary coverage build, whose anchor is the one tests/e2e/member-directory-freshness.spec.js asserts. It rides here rather than in a build of its own because this build already holds metrics.generatedAt present, and ReferenceArchitectures returns at its revision guard before it reads repository, so clearing the field un-covers nothing this build reaches and adds no compile to the end-to-end coverage job.", "set": { - "sources.architectures.revision": null + "sources.architectures.revision": null, + "sources.architectures.repository": null } } diff --git a/tests/e2e/member-directory-freshness-unlinked.spec.js b/tests/e2e/member-directory-freshness-unlinked.spec.js new file mode 100644 index 00000000..79045753 --- /dev/null +++ b/tests/e2e/member-directory-freshness-unlinked.spec.js @@ -0,0 +1,147 @@ +// End-to-end coverage for the plain-text arms of the provenance note on +// /community/members, which render when a source carries a date but no URL. +// +// DirectoryFreshness links each named source through a ternary +// (src/components/MemberDirectory/DirectoryFreshness.js): +// +// 25 {landscape?.sourceUrl ? ( +// 26 cncf/landscape +// 29 ) : ( +// 30 'cncf/landscape' +// 31 )} +// +// 39 {architectures?.repository ? ( +// 40 cncf/architecture +// 43 ) : ( +// 44 'cncf/architecture' +// 45 )} +// +// The right-hand arms at lines 30 and 44 are ordinary shapes rather than +// defensive dead code: data/members.json's sources.landscape is copied from +// the pinned landscape snapshot by scripts/generate-members.mjs, and +// data/metrics.json's sources.architectures is written by +// `npm run collect:metrics`. Either collector can pin a source it cannot +// address by URL, and the component is written to name the source as text +// rather than emit an empty anchor. tests/e2e/member-directory-freshness.spec.js +// already asserts the other side -- that both URLs are present and absolute -- +// and says in so many words that "the component falls back to plain text when +// a source carries no URL", but the committed data always carries both URLs, +// so no browser has ever run the fallback. +// +// Why this rides the no-revision build. Each arm needs its sentence to render +// -- a parseable timestamp -- while its URL is absent, and that pair conflicts +// with both existing builds: +// +// * the ordinary coverage build is where the anchors at lines 26 and 40 are +// measured from, so clearing the URLs there would trade those arms for +// these rather than add a case; +// * tests/e2e/fixtures/data-variant/ makes sources.landscape.collectedAt +// unparseable and clears metrics.generatedAt, so on that page neither +// sentence renders at all and the ternaries are never evaluated. +// +// tests/e2e/fixtures/data-no-revision/ holds exactly the shape these arms +// need: it leaves metrics.generatedAt and the landscape timestamp alone, so +// the note renders both sentences, and ReferenceArchitectures -- the component +// that build exists for -- returns at its revision guard before it reads +// sources.architectures.repository. Clearing the two URLs there un-covers +// nothing that build reaches. It is hosted there rather than in a directory of +// its own because each fixture directory is a full Docusaurus compile in the +// "End-to-end coverage" job (see the preamble of +// tests/tools/e2e-data-fixtures.cjs): a new build is for a shape that +// conflicts with every existing one, and this shape does not. +// +// The real route is asserted beside the no-revision one. An assertion that a +// page has no link passes just as well when the page failed to build or the +// selector is wrong; the pair is what makes it evidence that the ternary +// switched arms. +import { test, expect } from '../tools/e2e-coverage.cjs'; +import { loadSiteData } from '../tools/e2e-data-fixtures.cjs'; + +const MEMBERS_PATH = '/community/members'; +const NO_REVISION_BASE = '/e2e-coverage-no-revision'; + +const COVERAGE_ENV = { E2E_COVERAGE: '1' }; +const NO_REVISION_ENV = { + E2E_COVERAGE: '1', + E2E_COVERAGE_BUILD: 'no-revision', +}; + +// Read through the overlays rather than hard-coded, so an edited fixture fails +// here instead of leaving a test that asserts nothing. Each document is loaded +// with the env of the build whose page it is asserted against. +const members = loadSiteData('members.json', COVERAGE_ENV); +const metrics = loadSiteData('metrics.json', COVERAGE_ENV); +const unlinkedMembers = loadSiteData('members.json', NO_REVISION_ENV); +const unlinkedMetrics = loadSiteData('metrics.json', NO_REVISION_ENV); + +const MEMBERSHIP_SENTENCE = /Directory membership data last synced from/; +const ARCHITECTURES_SENTENCE = /Architecture-derived profiles last synced from/; + +const note = (page) => page.locator('p[class*="freshnessNote"]'); + +// The no-revision site is only built by the coverage run; outside it the base +// path does not exist. +const describeCoverage = + process.env.E2E_COVERAGE === '1' ? test.describe : test.describe.skip; + +describeCoverage('member directory freshness without source URLs', () => { + test('each source is named as plain text when it carries no URL', async ({ + page, + }) => { + // The shapes the two pages are built from. Stated as assertions rather + // than used to skip: an overlay that stopped clearing a URL would leave + // the arms below unexercised, which is the silent retirement this pair + // exists to catch. + expect(members?.sources?.landscape?.sourceUrl).toMatch(/^https:\/\//); + expect(metrics?.sources?.architectures?.repository).toMatch(/^https:\/\//); + expect(unlinkedMembers?.sources?.landscape?.sourceUrl).toBe(null); + expect(unlinkedMetrics?.sources?.architectures?.repository).toBe(null); + // Both sentences have to render on the no-revision page, or the ternaries + // are never reached and an absent link would prove nothing. + expect(unlinkedMembers?.sources?.landscape?.collectedAt).toEqual( + members?.sources?.landscape?.collectedAt, + ); + expect(unlinkedMetrics?.generatedAt).toEqual(metrics?.generatedAt); + + await page.goto(MEMBERS_PATH); + for (const name of ['cncf/landscape', 'cncf/architecture']) { + await expect( + note(page).getByRole('link', { name, exact: true }), + ).toHaveCount(1); + } + + await page.goto(`${NO_REVISION_BASE}${MEMBERS_PATH}`); + // The note still renders both sentences -- this is two links becoming + // text, not a sentence being dropped or the component returning null. + await expect(note(page)).toHaveCount(1); + await expect(note(page)).toContainText(MEMBERSHIP_SENTENCE); + await expect(note(page)).toContainText(ARCHITECTURES_SENTENCE); + + for (const name of ['cncf/landscape', 'cncf/architecture']) { + // The source is still named, so the sentence reads as prose rather than + // losing the source it is about... + await expect(note(page)).toContainText(name); + // ...and it is named as text, not as an anchor with an empty href, + // which is what the ternary exists to avoid. + await expect( + note(page).getByRole('link', { name, exact: true }), + ).toHaveCount(0); + } + await expect(note(page).getByRole('link')).toHaveCount(0); + }); + + test('the directory itself is unaffected by the unlinked sources', async ({ + page, + }) => { + await page.goto(`${NO_REVISION_BASE}${MEMBERS_PATH}`); + + // A member card list proves the page is intact, so the assertions above + // are about the note and not about a build that lost the route. + const section = page.getByRole('region', { + name: 'End User Community organization directory', + }); + await expect( + section.getByRole('button', { name: /^Open .+ profile$/ }).first(), + ).toBeVisible(); + }); +});