visitAllLinks: production-faithful navigation for relative and hash links; linear crawl

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');
+  });
 });

test-app/app/templates/application.gts

@@ -7,6 +7,10 @@
   <a href="/does-not-exist">here</a>
   <a href="#title">here</a>
   <a href="/#title">/ here</a>
+  <a href="/docs/page">a nested page (with a relative link on it)</a>
+  {{! a page+hash link: currentURL() after the click has no #hash, so the
+      navigation assertion must compare without it }}
+  <a href="/hash-target#down">hash link</a>
 
   {{! non-SPA links: handled by the browser, not the router — visitAllLinks must skip them }}
   <a href="/foo" target="_blank" rel="noopener noreferrer">new tab</a>

test-app/app/templates/docs/other.gts

@@ -0,0 +1,3 @@
+<template>
+  <h3>the relative link target</h3>
+</template>

test-app/app/templates/docs/page.gts

@@ -0,0 +1,10 @@
+<template>
+  <h3>a nested docs page</h3>
+
+  {{! a RELATIVE href: from /docs/page this must navigate to /docs/other.
+      In the test harness the browser resolves element.href against the test
+      page's URL (/tests), which would send the router to /other instead —
+      visitAllLinks points the anchor at the currentURL-resolved target
+      before clicking. }}
+  <a href="other">relative sibling</a>
+</template>

test-app/app/templates/hash-target.gts

@@ -0,0 +1,5 @@
+<template>
+  <h3>hash target</h3>
+
+  <p id="down">the anchor destination</p>
+</template>

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 `<a href="other">`, 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(', ')}`,
+    );
+  });
 });

test-support/src/routing/visit-all.ts

@@ -69,7 +69,7 @@ export async function visitAllLinks(
   knownRedirects?: Record<string, string>,
 ) {
   /**
-   * 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);