diff --git a/test-app/app/router.ts b/test-app/app/router.ts index a5eeddf..4ced252 100644 --- a/test-app/app/router.ts +++ b/test-app/app/router.ts @@ -12,4 +12,9 @@ export default class Router extends EmberRouter { Router.map(function () { this.route('foo'); + this.route('hash-target'); + this.route('docs', function () { + this.route('page'); + this.route('other'); + }); }); diff --git a/test-app/app/templates/application.gts b/test-app/app/templates/application.gts index a8daeb3..67b4bc2 100644 --- a/test-app/app/templates/application.gts +++ b/test-app/app/templates/application.gts @@ -7,6 +7,10 @@ here here / here + a nested page (with a relative link on it) + {{! a page+hash link: currentURL() after the click has no #hash, so the + navigation assertion must compare without it }} + hash link {{! non-SPA links: handled by the browser, not the router — visitAllLinks must skip them }} new tab diff --git a/test-app/app/templates/docs/other.gts b/test-app/app/templates/docs/other.gts new file mode 100644 index 0000000..ce4536b --- /dev/null +++ b/test-app/app/templates/docs/other.gts @@ -0,0 +1,3 @@ + diff --git a/test-app/app/templates/docs/page.gts b/test-app/app/templates/docs/page.gts new file mode 100644 index 0000000..d20ce6c --- /dev/null +++ b/test-app/app/templates/docs/page.gts @@ -0,0 +1,10 @@ + diff --git a/test-app/app/templates/hash-target.gts b/test-app/app/templates/hash-target.gts new file mode 100644 index 0000000..b94e1ed --- /dev/null +++ b/test-app/app/templates/hash-target.gts @@ -0,0 +1,5 @@ + diff --git a/test-app/tests/acceptance/visit-all-links-test.ts b/test-app/tests/acceptance/visit-all-links-test.ts index 1bfa216..b799a8b 100644 --- a/test-app/tests/acceptance/visit-all-links-test.ts +++ b/test-app/tests/acceptance/visit-all-links-test.ts @@ -39,4 +39,56 @@ module('All Links', function (hooks) { assert.ok(visited.length > 0, 'the SPA links were still visited'); }); + + test('relative links navigate against the current route, and warn', async function (assert) { + // /docs/page renders ``, which from /docs/page must land + // on /docs/other. The browser resolves the anchor's element.href against + // the TEST PAGE's url (/tests) — which would send the router to /other — + // so the crawler points the anchor at the currentURL-resolved target + // before clicking (and warns: relative hrefs are an authoring hazard the + // app author should fix). The navigation assertions inside visitAllLinks + // fail this test if the click lands anywhere else. + const visited: string[] = []; + const warnings: string[] = []; + const originalWarn = console.warn; + + console.warn = (...args: unknown[]) => warnings.push(args.join(' ')); + + try { + await visitAllLinks((url) => { + visited.push(url); + }); + } finally { + console.warn = originalWarn; + } + + assert.true(visited.includes('/docs/other'), `visited: ${visited.join(', ')}`); + + const warning = warnings.find((w) => w.includes('Relative href "other"')); + + assert.true(Boolean(warning), 'warned about the relative href'); + assert.true( + Boolean(warning?.includes('"/docs/other"')), + 'the warning names the resolved target to author instead', + ); + }); + + test('each target is visited once', async function (assert) { + // `visited` is keyed on the target path alone (not (page, target) pairs): + // shared links — like this app's application-template nav — appear on + // every page, and pair-keying makes the crawl quadratic in app size. + const visited: string[] = []; + + await visitAllLinks((url) => { + visited.push(url); + }); + + const paths = visited.map((url) => url.split('#')[0]); + + assert.strictEqual( + new Set(paths).size, + paths.length, + `no target is visited twice: ${paths.join(', ')}`, + ); + }); }); diff --git a/test-support/src/routing/visit-all.ts b/test-support/src/routing/visit-all.ts index 2a1febd..29ce70a 100644 --- a/test-support/src/routing/visit-all.ts +++ b/test-support/src/routing/visit-all.ts @@ -69,7 +69,7 @@ export async function visitAllLinks( knownRedirects?: Record, ) { /** - * string of "on::target" + * app-relative target paths (without hash) */ const visited = new Set(); let returnTo = '/'; @@ -110,7 +110,12 @@ export async function visitAllLinks( continue; } - const key = `${currentURL()}::${nonHashPart}`; + // Keyed on the target alone: this crawl answers "is every reachable URL + // visitable?", so one visit per target suffices. Keying on + // (current page, target) pairs re-visits every target from every page + // that links it — quadratic in the size of the app (shared nav links + // appear on every page), which times out on documentation-sized apps. + const key = nonHashPart; if (visited.has(key)) continue; @@ -130,16 +135,40 @@ export async function visitAllLinks( debugAssert(`link exists via selector \`${toVisit.selector}\``, link); + /** + * The click navigates by `element.href`, which the browser resolved + * against the test page's URL (e.g. `/tests`) — NOT against the app's + * current route the way a production visit would (there, the address bar + * is the current route). A relative href would therefore navigate + * somewhere the real app never goes. We already resolved the target + * against `currentURL()` when the link was encountered, so point the + * anchor at that; the click then exercises the real + * properLinks-and-router path with the production URL. + */ + if (!toVisit.original.startsWith('/')) { + console.warn( + `[visitAllLinks] Relative href "${toVisit.original}" found on ${returnTo}. ` + + `Relative hrefs resolve against the browser's URL rather than the app's current route, ` + + `so they only behave in a real full-page visit — they misresolve in this test harness ` + + `and under any mount where the address bar isn't the route (embeds, previews). ` + + `The crawler pointed this click at the resolved target instead. ` + + `Action: update the source document to link to "${toVisit.href}" directly.`, + ); + link.setAttribute('href', toVisit.href); + } + await click(link); const current = rootURL.replace(/\/$/, '') + '/' + currentURL().replace(/^\//, ''); const expected = knownRedirects?.[toVisit.href] ?? toVisit.href; + // currentURL() never includes a #hash, so compare without it + const [expectedPath] = expected.split('#'); assert.pushResult({ - result: current.startsWith(expected), + result: current.startsWith(expectedPath ?? expected), actual: current, - expected: expected, + expected: expectedPath, message: `Navigation was successful: to:${toVisit.original}, from:${returnTo}`, }); visited.add(key);