From a751279ffe80c8fc0f1068fd599efcf8a3a78b0c Mon Sep 17 00:00:00 2001 From: quality Date: Sun, 4 Oct 2026 10:43:17 -0400 Subject: [PATCH] test: cover websiteUrl's bare-host, unparseable and absent arms end to end src/lib/profile-links.mjs resolves the free-text blog field of a third-party GitHub profile into an href. Three of its arms are unit-covered but unreachable in a browser against the checked-in data: the typeof guard, the no-scheme arm that resolves a bare host against https rather than concatenating it, and the URL-parser catch. Adds three staff records to the E2E_COVERAGE=1 data overlay carrying those shapes, and asserts each rendered outcome in tests/e2e/data-fixtures.spec.js. npm run build:production is unchanged: the overlay is applied only by the coverage build's loader. Closes #1053 Signed-off-by: quality --- tests/e2e/data-fixtures.spec.js | 70 +++++++++++++++++++ tests/e2e/fixtures/data/community-people.json | 38 +++++++++- 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/tests/e2e/data-fixtures.spec.js b/tests/e2e/data-fixtures.spec.js index 910b47e8..7c8b5915 100644 --- a/tests/e2e/data-fixtures.spec.js +++ b/tests/e2e/data-fixtures.spec.js @@ -41,6 +41,14 @@ const groups = loadSiteData('community-groups.json').groups; const stale = groups.filter((group) => group.archived || !group.reachable); const people = loadSiteData('community-people.json').people.staff || []; const anonymous = people.find((person) => !person.role && !person.company); +const byName = (name) => people.find((person) => person.name === name); +// websiteUrl() resolves the free-text "blog" field of a third-party GitHub +// profile. Each fixture below holds open one arm of that resolution; they are +// addressed through the overlay so a renamed or dropped record fails here +// rather than quietly stopping the coverage it was added for. +const absentWebsite = byName('Coverage Fixture Absent Website'); +const bareHostWebsite = byName('Coverage Fixture Bare Host Website'); +const unparseableWebsite = byName('Coverage Fixture Unparseable Website'); const members = loadSiteData('members.json').members; const dualRole = members.find( (member) => member.membershipStatus === 'member-and-contributor', @@ -60,6 +68,17 @@ async function waitForHydration(locator) { }); } +async function openStaffProfile(page, name) { + await page.goto('/community'); + const trigger = page.getByRole('button', { name: `Open ${name} profile` }); + await waitForHydration(trigger); + await trigger.click(); + const dialog = page.getByRole('dialog'); + await expect(dialog).toBeVisible(); + await expect(dialog.locator('#profile-name')).toHaveText(name); + return dialog; +} + describeCoverage('drifted upstream data', () => { test('GroupLinkStatus warns about an archived or unreachable group', async ({ page, @@ -96,6 +115,57 @@ describeCoverage('drifted upstream data', () => { ).toHaveCount(1); }); + test('PersonDialog resolves a website given as a bare host', async ({ + page, + }) => { + expect(bareHostWebsite).toBeTruthy(); + expect(bareHostWebsite.blog).toBe('fixture.example'); + + const dialog = await openStaffProfile(page, bareHostWebsite.name); + + // The value carries no scheme, so websiteUrl resolves it against https + // rather than concatenating it: the rendered href is an absolute URL whose + // authority is the value itself, which is what stops a value such as + // "trusted.example@attacker.example" from linking somewhere else. + const website = dialog.getByRole('link', { name: 'Website' }); + await expect(website).toHaveAttribute( + 'href', + `https://${bareHostWebsite.blog}/`, + ); + }); + + test('PersonDialog drops a website that parses as no URL at all', async ({ + page, + }) => { + expect(unparseableWebsite).toBeTruthy(); + expect(unparseableWebsite.blog).toBe('https://['); + // The value opens with a scheme, so websiteUrl passes it to the URL parser + // unchanged; the parser is what rejects it. + expect(() => new URL(unparseableWebsite.blog)).toThrow(); + + const dialog = await openStaffProfile(page, unparseableWebsite.name); + + // The links array is filtered on href, so an unresolvable value renders no + // anchor rather than a dead one. + await expect(dialog.getByRole('link', { name: 'Website' })).toHaveCount(0); + await expect(dialog.locator('a[href]')).toHaveCount(0); + }); + + test('PersonDialog drops a website the profile does not carry', async ({ + page, + }) => { + expect(absentWebsite).toBeTruthy(); + expect(absentWebsite.blog).toBe(null); + + const dialog = await openStaffProfile(page, absentWebsite.name); + + // A non-string reaches websiteUrl before it is trimmed, so the guard that + // rejects it is a different one from the empty-string case the role-less + // fixture above covers. + await expect(dialog.getByRole('link', { name: 'Website' })).toHaveCount(0); + await expect(dialog.locator('a[href]')).toHaveCount(0); + }); + test('MemberCard labels an organization holding both membership roles', async ({ page, }) => { diff --git a/tests/e2e/fixtures/data/community-people.json b/tests/e2e/fixtures/data/community-people.json index bb8c4f3c..9e5eb054 100644 --- a/tests/e2e/fixtures/data/community-people.json +++ b/tests/e2e/fixtures/data/community-people.json @@ -1,5 +1,5 @@ { - "description": "Adds one staff entry with neither role nor company so PersonDialog falls back to 'Community member' (src/components/CommunityPeople/index.js line 63). All real people carry a role or a company. Appended to 'staff' rather than 'tab' so the TAB roster assertions in tests/e2e/community-people.spec.js keep describing the real data.", + "description": "Adds staff entries for four branches no real profile reaches. The first has neither role nor company, so PersonDialog falls back to 'Community member' (src/components/CommunityPeople/index.js line 63). The remaining three carry the three website shapes websiteUrl (src/lib/profile-links.mjs) resolves differently and data/community-people.json never contains: an absent blog (the typeof guard at line 51), a bare host with no scheme (the ternary's no-scheme arm at line 55, resolved against https rather than concatenated), and a value that parses as a scheme but not as a URL (the catch at line 62). The blog field is copied verbatim from a third-party GitHub profile, so every one of those shapes is an input the next refresh can deliver. Appended to 'staff' rather than 'tab' so the TAB roster assertions in tests/e2e/community-people.spec.js keep describing the real data; all three carry a role and a company so they cannot displace the role-less entry above.", "append": { "people.staff": [ { @@ -13,6 +13,42 @@ "linkedin": "", "twitter": "", "blog": "" + }, + { + "name": "Coverage Fixture Absent Website", + "company": "Coverage Fixture Org", + "role": "Fixture", + "bio": "", + "location": "", + "image": "/img/favicon.svg", + "github": "", + "linkedin": "", + "twitter": "", + "blog": null + }, + { + "name": "Coverage Fixture Bare Host Website", + "company": "Coverage Fixture Org", + "role": "Fixture", + "bio": "", + "location": "", + "image": "/img/favicon.svg", + "github": "", + "linkedin": "", + "twitter": "", + "blog": "fixture.example" + }, + { + "name": "Coverage Fixture Unparseable Website", + "company": "Coverage Fixture Org", + "role": "Fixture", + "bio": "", + "location": "", + "image": "/img/favicon.svg", + "github": "", + "linkedin": "", + "twitter": "", + "blog": "https://[" } ] }