end-to-end test link rewriting plugin (github#34939)

.github/workflows/test.yml

@@ -115,6 +115,20 @@ jobs:
         if: ${{ matrix.test-group == 'rendering-fixtures' }}
         run: ./script/copy-fixture-data.js --check
 
+      # This keeps our fixture content/data in check
+      - name: Check the test fixture content (if applicable)
+        if: ${{ matrix.test-group == 'rendering-fixtures' }}
+        env:
+          ROOT: tests/fixtures
+        run: |
+          # If either of these fail, it means our fixture content's internal
+          # links can and should be updated.
+          ./script/update-internal-links.js --dry-run --check --strict --verbose \
+            tests/fixtures/content \
+            --exclude tests/fixtures/content/get-started/foo/typo-autotitling.md
+          ./script/update-internal-links.js --dry-run --check --strict --verbose \
+            tests/fixtures/data
+
       - name: Clone all translations
         if: ${{ matrix.test-group == 'translations' }}
         uses: ./.github/actions/clone-translations

script/update-internal-links.js

@@ -29,6 +29,7 @@ program
   .option('--check', 'Exit and fail if it found something to fix')
   .option('--aggregate-stats', 'Display aggregate numbers about all possible changes')
   .option('--strict', "Throw an error (instead of a warning) if a link can't be processed")
+  .option('--exclude [paths...]', 'Specific files to exclude')
   .arguments('[files-or-directories...]', '')
   .parse(process.argv)
 
@@ -37,6 +38,8 @@ main(program.args, program.opts())
 async function main(files, opts) {
   const { debug } = opts
 
+  const excludeFilePaths = new Set(opts.exclude || [])
+
   try {
     if (opts.check && !opts.dryRun) {
       throw new Error("Can't use --check without --dry-run")
@@ -47,15 +50,25 @@ async function main(files, opts) {
       files.push('content', 'data')
     }
     for (const file of files) {
-      if (!(file.startsWith('content') || file.startsWith('data'))) {
+      if (
+        !(
+          file.startsWith('content') ||
+          file.startsWith('data') ||
+          file.startsWith('tests/fixtures')
+        )
+      ) {
         throw new Error(`${file} must be a content or data filepath`)
       }
       if (!fs.existsSync(file)) {
         throw new Error(`${file} does not exist`)
       }
       if (fs.lstatSync(file).isDirectory()) {
-        actualFiles.push(...walkFiles(file, ['.md', '.yml']))
-      } else {
+        actualFiles.push(
+          ...walkFiles(file, ['.md', '.yml']).filter((p) => {
+            return !excludeFilePaths.has(p)
+          })
+        )
+      } else if (!excludeFilePaths.has(file)) {
         actualFiles.push(file)
       }
     }

tests/fixtures/content/get-started/quickstart/index.md

@@ -8,4 +8,5 @@ versions:
   ghec: '*'
 children:
   - /hello-world
+  - /link-rewriting
 ---

tests/fixtures/content/get-started/quickstart/link-rewriting.md

@@ -0,0 +1,30 @@
+---
+title: Link rewriting
+intro: 'When rendered, links are rewritting to perfection for viewing'
+versions:
+  fpt: '*'
+  ghes: '*'
+  ghae: '*'
+  ghec: '*'
+type: quick_start
+---
+
+## Internal links never need language prefix
+
+[AUTOTITLE](/get-started/foo/cross-version-linking) already tests things
+like `/enterprise-server@latest/` becomes `/enterprise-server@X.Y/` where
+`X.Y` is the latest Enterprise server version.
+
+## External links are left alone
+
+[@peterbe](https://github.com/peterbe)
+
+## Legacy enterprise links
+
+For example, [this Enterprise 11.10 link](/enterprise/11.10.340/admin/articles/upgrading-to-the-latest-release).
+
+## Links to `assets` and `public`
+
+Here's a [Picture](/assets/images/_fixtures/screenshot.png).
+
+[A GraphQL Schema](/public/schema.docs.graphql)

tests/rendering-fixtures/internal-links.js

@@ -40,18 +40,68 @@ describe('cross-version-links', () => {
       const links = $('#article-contents a[href]')
 
       // Tests that the hardcoded prefix is always removed
-      const firstLink = links.filter(function () {
-        return $(this).text() === 'Hello world always in free-pro-team'
-      })
+      const firstLink = links.filter(
+        (i, element) => $(element).text() === 'Hello world always in free-pro-team'
+      )
       expect(firstLink.attr('href')).toBe('/en/get-started/quickstart/hello-world')
 
       // Tests that the second link always goes to enterprise-server@X.Y
-      const secondLink = links.filter(function () {
-        return $(this).text() === 'Autotitling page always in enterprise-server latest'
-      })
+      const secondLink = links.filter(
+        (i, element) => $(element).text() === 'Autotitling page always in enterprise-server latest'
+      )
       expect(secondLink.attr('href')).toBe(
         `/en/enterprise-server@${enterpriseServerReleases.latest}/get-started/quickstart/hello-world`
       )
     }
   )
 })
+
+describe('link-rewriting', () => {
+  test('/en is injected', async () => {
+    const $ = await getDOM('/get-started/quickstart/link-rewriting')
+    const links = $('#article-contents a[href]')
+
+    {
+      const link = links.filter((i, element) => $(element).text() === 'Cross Version Linking')
+      expect(link.attr('href')).toMatch('/en/get-started/')
+    }
+
+    // Some links are left untouched
+
+    {
+      const link = links.filter((i, element) => $(element).text().includes('Enterprise 11.10'))
+      expect(link.attr('href')).toMatch('/en/enterprise/')
+    }
+    {
+      const link = links.filter((i, element) => $(element).text().includes('peterbe'))
+      expect(link.attr('href')).toMatch(/^https:/)
+    }
+    {
+      const link = links.filter((i, element) => $(element).text().includes('Picture'))
+      expect(link.attr('href')).toMatch(/^\/assets\//)
+    }
+    {
+      const link = links.filter((i, element) => $(element).text().includes('GraphQL Schema'))
+      expect(link.attr('href')).toMatch(/^\/public\//)
+    }
+  })
+
+  test('/en and current version (latest) is injected', async () => {
+    const $ = await getDOM('/enterprise-cloud@latest/get-started/quickstart/link-rewriting')
+    const links = $('#article-contents a[href]')
+
+    const link = links.filter((i, element) => $(element).text() === 'Cross Version Linking')
+    expect(link.attr('href')).toMatch('/en/enterprise-cloud@latest/get-started/')
+  })
+
+  test('/en and current version number is injected', async () => {
+    // enterprise-server, unlike enterprise-cloud, use numbers
+    const $ = await getDOM('/enterprise-server@latest/get-started/quickstart/link-rewriting')
+    const links = $('#article-contents a[href]')
+
+    const link = links.filter((i, element) => $(element).text() === 'Cross Version Linking')
+    expect(link.attr('href')).toMatch(
+      `/en/enterprise-server@${enterpriseServerReleases.latest}/get-started/`
+    )
+  })
+})

tests/unit/page.js

@@ -68,140 +68,7 @@ describe('Page class', () => {
     })
   })
 
-  describe.skip('page.render(context)', () => {
-    test('rewrites links to include the current language prefix and version', async () => {
-      const page = await Page.init(opts)
-      const context = {
-        page: { version: `enterprise-server@${enterpriseServerReleases.latest}` },
-        currentVersion: `enterprise-server@${enterpriseServerReleases.latest}`,
-        currentPath:
-          '/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-branches',
-        currentLanguage: 'en',
-      }
-      const rendered = await page.render(context)
-      const $ = cheerio.load(rendered)
-      expect(
-        page.markdown.includes(
-          '(/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests)'
-        )
-      ).toBe(true)
-      expect(
-        page.markdown.includes(
-          '(/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests)'
-        )
-      ).toBe(false)
-      expect(
-        $(
-          'a[href="/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests"]'
-        ).length
-      ).toBe(0)
-      expect(
-        $(
-          `a[href="/en/${`enterprise-server@${enterpriseServerReleases.latest}`}/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests"]`
-        ).length
-      ).toBeGreaterThan(0)
-    })
-
-    // Much of this test is based on making sure we don't
-    // repeat the bug introduced in issue 1545.
-    test('rewrites links correctly for unsupported enterprise-server links', async () => {
-      const page = await Page.init({
-        relativePath: 'page-with-deprecated-enterprise-links.md',
-        basePath: path.join(__dirname, '../fixtures'),
-        languageCode: 'en',
-      })
-      const context = {
-        page: { version: `enterprise-server@${enterpriseServerReleases.latest}` },
-        currentVersion: `enterprise-server@${enterpriseServerReleases.latest}`,
-        currentPath: '/en/page-with-deprecated-enterprise-links',
-        currentLanguage: 'en',
-      }
-      const rendered = await page.render(context)
-      // That page only contains exactly 2 links. And we can know
-      // exactly what we expect each one to be.
-      const $ = cheerio.load(rendered)
-      const first = $('a[href]').first()
-      expect(first.text()).toBe('Version 2.22')
-      expect(first.attr('href')).toBe('/en/enterprise-server@2.22')
-      const last = $('a[href]').last()
-      expect(last.text()).toBe('Version 3.2')
-      expect(last.attr('href')).toBe('/en/enterprise-server@3.2')
-    })
-
-    test('rewrites links in the intro to include the current language prefix and version', async () => {
-      const page = await Page.init(opts)
-      page.rawIntro =
-        '[Pull requests](/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests)'
-      const context = {
-        page: { version: nonEnterpriseDefaultVersion },
-        currentVersion: nonEnterpriseDefaultVersion,
-        currentPath:
-          '/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-branches',
-        currentLanguage: 'en',
-      }
-      // This is needed because unit tests are weird. The page.render()
-      // method is dependent on module global cache.
-      // We need to fudge the `currentPath` so it appears to be different.
-      context.currentPath += Math.random()
-      await page.render(context)
-      const $ = cheerio.load(page.intro)
-      expect(
-        $(
-          'a[href="/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests"]'
-        ).length
-      ).toBe(0)
-      expect(
-        $(
-          'a[href="/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests"]'
-        ).length
-      ).toBeGreaterThan(0)
-    })
-
-    test('does not rewrite links that include deprecated enterprise release numbers', async () => {
-      const page = await Page.init({
-        relativePath:
-          'admin/enterprise-management/updating-the-virtual-machine-and-physical-resources/migrating-from-github-enterprise-1110x-to-2123.md',
-        basePath: path.join(__dirname, '../../content'),
-        languageCode: 'en',
-      })
-      const context = {
-        page: { version: `enterprise-server@${enterpriseServerReleases.latest}` },
-        currentVersion: `enterprise-server@${enterpriseServerReleases.latest}`,
-        currentPath: `/en/enterprise-server@${enterpriseServerReleases.latest}/admin/enterprise-management/migrating-from-github-enterprise-1110x-to-2123`,
-        currentLanguage: 'en',
-      }
-      const rendered = await page.render(context)
-      const $ = cheerio.load(rendered)
-      expect(
-        page.markdown.includes(
-          '(/enterprise/11.10.340/admin/articles/upgrading-to-the-latest-release/)'
-        )
-      ).toBe(true)
-      expect(
-        $(
-          `a[href="/en/enterprise-server@${enterpriseServerReleases.latest}/11.10.340/admin/articles/upgrading-to-the-latest-release"]`
-        ).length
-      ).toBe(0)
-      expect(
-        $('a[href="/en/enterprise/11.10.340/admin/articles/upgrading-to-the-latest-release"]')
-          .length
-      ).toBeGreaterThan(0)
-    })
-
-    test('does not rewrite links to external redirects', async () => {
-      const page = await Page.init(opts)
-      page.markdown = `${page.markdown}\n\nSee [Capistrano](/capistrano).`
-      const context = {
-        page: { version: nonEnterpriseDefaultVersion },
-        currentVersion: nonEnterpriseDefaultVersion,
-        currentPath: `/en/${nonEnterpriseDefaultVersion}/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-branches`,
-        currentLanguage: 'en',
-      }
-      const rendered = await page.render(context)
-      const $ = cheerio.load(rendered)
-      expect($('a[href="/capistrano"]').length).toBe(1)
-    })
-
+  describe('page.render(context)', () => {
     // Most of our Liquid versioning tests are in https://github.com/docs/render-content,
     // But they don't have access to our currently supported versions, which we're testing here.
     // This test ensures that this works as expected: {% if enterpriseServerVersions contains currentVersion %}