Skip to content

Commit 2fa45f9

Browse files
author
Peter Bengtsson
authored
end-to-end test link rewriting plugin (github#34939)
1 parent 8c04f7a commit 2fa45f9

7 files changed

Lines changed: 118 additions & 154 deletions

File tree

‎.github/workflows/test.yml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,20 @@ jobs:
115115
if: ${{ matrix.test-group == 'rendering-fixtures' }}
116116
run: ./script/copy-fixture-data.js --check
117117

118+
# This keeps our fixture content/data in check
119+
- name: Check the test fixture content (if applicable)
120+
if: ${{ matrix.test-group == 'rendering-fixtures' }}
121+
env:
122+
ROOT: tests/fixtures
123+
run: |
124+
# If either of these fail, it means our fixture content's internal
125+
# links can and should be updated.
126+
./script/update-internal-links.js --dry-run --check --strict --verbose \
127+
tests/fixtures/content \
128+
--exclude tests/fixtures/content/get-started/foo/typo-autotitling.md
129+
./script/update-internal-links.js --dry-run --check --strict --verbose \
130+
tests/fixtures/data
131+
118132
- name: Clone all translations
119133
if: ${{ matrix.test-group == 'translations' }}
120134
uses: ./.github/actions/clone-translations

‎script/update-internal-links.js‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ program
2929
.option('--check', 'Exit and fail if it found something to fix')
3030
.option('--aggregate-stats', 'Display aggregate numbers about all possible changes')
3131
.option('--strict', "Throw an error (instead of a warning) if a link can't be processed")
32+
.option('--exclude [paths...]', 'Specific files to exclude')
3233
.arguments('[files-or-directories...]', '')
3334
.parse(process.argv)
3435

@@ -37,6 +38,8 @@ main(program.args, program.opts())
3738
async function main(files, opts) {
3839
const { debug } = opts
3940

41+
const excludeFilePaths = new Set(opts.exclude || [])
42+
4043
try {
4144
if (opts.check && !opts.dryRun) {
4245
throw new Error("Can't use --check without --dry-run")
@@ -47,15 +50,25 @@ async function main(files, opts) {
4750
files.push('content', 'data')
4851
}
4952
for (const file of files) {
50-
if (!(file.startsWith('content') || file.startsWith('data'))) {
53+
if (
54+
!(
55+
file.startsWith('content') ||
56+
file.startsWith('data') ||
57+
file.startsWith('tests/fixtures')
58+
)
59+
) {
5160
throw new Error(`${file} must be a content or data filepath`)
5261
}
5362
if (!fs.existsSync(file)) {
5463
throw new Error(`${file} does not exist`)
5564
}
5665
if (fs.lstatSync(file).isDirectory()) {
57-
actualFiles.push(...walkFiles(file, ['.md', '.yml']))
58-
} else {
66+
actualFiles.push(
67+
...walkFiles(file, ['.md', '.yml']).filter((p) => {
68+
return !excludeFilePaths.has(p)
69+
})
70+
)
71+
} else if (!excludeFilePaths.has(file)) {
5972
actualFiles.push(file)
6073
}
6174
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ versions:
88
ghec: '*'
99
children:
1010
- /hello-world
11+
- /link-rewriting
1112
---

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
title: Link rewriting
3+
intro: 'When rendered, links are rewritting to perfection for viewing'
4+
versions:
5+
fpt: '*'
6+
ghes: '*'
7+
ghae: '*'
8+
ghec: '*'
9+
type: quick_start
10+
---
11+
12+
## Internal links never need language prefix
13+
14+
[AUTOTITLE](/get-started/foo/cross-version-linking) already tests things
15+
like `/enterprise-server@latest/` becomes `/enterprise-server@X.Y/` where
16+
`X.Y` is the latest Enterprise server version.
17+
18+
## External links are left alone
19+
20+
[@peterbe](https://github.com/peterbe)
21+
22+
## Legacy enterprise links
23+
24+
For example, [this Enterprise 11.10 link](/enterprise/11.10.340/admin/articles/upgrading-to-the-latest-release).
25+
26+
## Links to `assets` and `public`
27+
28+
Here's a [Picture](/assets/images/_fixtures/screenshot.png).
29+
30+
[A GraphQL Schema](/public/schema.docs.graphql)

‎tests/fixtures/page-with-deprecated-enterprise-links.md‎

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎tests/rendering-fixtures/internal-links.js‎

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,68 @@ describe('cross-version-links', () => {
4040
const links = $('#article-contents a[href]')
4141

4242
// Tests that the hardcoded prefix is always removed
43-
const firstLink = links.filter(function () {
44-
return $(this).text() === 'Hello world always in free-pro-team'
45-
})
43+
const firstLink = links.filter(
44+
(i, element) => $(element).text() === 'Hello world always in free-pro-team'
45+
)
4646
expect(firstLink.attr('href')).toBe('/en/get-started/quickstart/hello-world')
4747

4848
// Tests that the second link always goes to enterprise-server@X.Y
49-
const secondLink = links.filter(function () {
50-
return $(this).text() === 'Autotitling page always in enterprise-server latest'
51-
})
49+
const secondLink = links.filter(
50+
(i, element) => $(element).text() === 'Autotitling page always in enterprise-server latest'
51+
)
5252
expect(secondLink.attr('href')).toBe(
5353
`/en/enterprise-server@${enterpriseServerReleases.latest}/get-started/quickstart/hello-world`
5454
)
5555
}
5656
)
5757
})
58+
59+
describe('link-rewriting', () => {
60+
test('/en is injected', async () => {
61+
const $ = await getDOM('/get-started/quickstart/link-rewriting')
62+
const links = $('#article-contents a[href]')
63+
64+
{
65+
const link = links.filter((i, element) => $(element).text() === 'Cross Version Linking')
66+
expect(link.attr('href')).toMatch('/en/get-started/')
67+
}
68+
69+
// Some links are left untouched
70+
71+
{
72+
const link = links.filter((i, element) => $(element).text().includes('Enterprise 11.10'))
73+
expect(link.attr('href')).toMatch('/en/enterprise/')
74+
}
75+
{
76+
const link = links.filter((i, element) => $(element).text().includes('peterbe'))
77+
expect(link.attr('href')).toMatch(/^https:/)
78+
}
79+
{
80+
const link = links.filter((i, element) => $(element).text().includes('Picture'))
81+
expect(link.attr('href')).toMatch(/^\/assets\//)
82+
}
83+
{
84+
const link = links.filter((i, element) => $(element).text().includes('GraphQL Schema'))
85+
expect(link.attr('href')).toMatch(/^\/public\//)
86+
}
87+
})
88+
89+
test('/en and current version (latest) is injected', async () => {
90+
const $ = await getDOM('/enterprise-cloud@latest/get-started/quickstart/link-rewriting')
91+
const links = $('#article-contents a[href]')
92+
93+
const link = links.filter((i, element) => $(element).text() === 'Cross Version Linking')
94+
expect(link.attr('href')).toMatch('/en/enterprise-cloud@latest/get-started/')
95+
})
96+
97+
test('/en and current version number is injected', async () => {
98+
// enterprise-server, unlike enterprise-cloud, use numbers
99+
const $ = await getDOM('/enterprise-server@latest/get-started/quickstart/link-rewriting')
100+
const links = $('#article-contents a[href]')
101+
102+
const link = links.filter((i, element) => $(element).text() === 'Cross Version Linking')
103+
expect(link.attr('href')).toMatch(
104+
`/en/enterprise-server@${enterpriseServerReleases.latest}/get-started/`
105+
)
106+
})
107+
})

‎tests/unit/page.js‎

Lines changed: 1 addition & 134 deletions
Original file line numberDiff line numberDiff line change
@@ -68,140 +68,7 @@ describe('Page class', () => {
6868
})
6969
})
7070

71-
describe.skip('page.render(context)', () => {
72-
test('rewrites links to include the current language prefix and version', async () => {
73-
const page = await Page.init(opts)
74-
const context = {
75-
page: { version: `enterprise-server@${enterpriseServerReleases.latest}` },
76-
currentVersion: `enterprise-server@${enterpriseServerReleases.latest}`,
77-
currentPath:
78-
'/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-branches',
79-
currentLanguage: 'en',
80-
}
81-
const rendered = await page.render(context)
82-
const $ = cheerio.load(rendered)
83-
expect(
84-
page.markdown.includes(
85-
'(/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests)'
86-
)
87-
).toBe(true)
88-
expect(
89-
page.markdown.includes(
90-
'(/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests)'
91-
)
92-
).toBe(false)
93-
expect(
94-
$(
95-
'a[href="/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests"]'
96-
).length
97-
).toBe(0)
98-
expect(
99-
$(
100-
`a[href="/en/${`enterprise-server@${enterpriseServerReleases.latest}`}/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests"]`
101-
).length
102-
).toBeGreaterThan(0)
103-
})
104-
105-
// Much of this test is based on making sure we don't
106-
// repeat the bug introduced in issue 1545.
107-
test('rewrites links correctly for unsupported enterprise-server links', async () => {
108-
const page = await Page.init({
109-
relativePath: 'page-with-deprecated-enterprise-links.md',
110-
basePath: path.join(__dirname, '../fixtures'),
111-
languageCode: 'en',
112-
})
113-
const context = {
114-
page: { version: `enterprise-server@${enterpriseServerReleases.latest}` },
115-
currentVersion: `enterprise-server@${enterpriseServerReleases.latest}`,
116-
currentPath: '/en/page-with-deprecated-enterprise-links',
117-
currentLanguage: 'en',
118-
}
119-
const rendered = await page.render(context)
120-
// That page only contains exactly 2 links. And we can know
121-
// exactly what we expect each one to be.
122-
const $ = cheerio.load(rendered)
123-
const first = $('a[href]').first()
124-
expect(first.text()).toBe('Version 2.22')
125-
expect(first.attr('href')).toBe('/en/enterprise-server@2.22')
126-
const last = $('a[href]').last()
127-
expect(last.text()).toBe('Version 3.2')
128-
expect(last.attr('href')).toBe('/en/enterprise-server@3.2')
129-
})
130-
131-
test('rewrites links in the intro to include the current language prefix and version', async () => {
132-
const page = await Page.init(opts)
133-
page.rawIntro =
134-
'[Pull requests](/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests)'
135-
const context = {
136-
page: { version: nonEnterpriseDefaultVersion },
137-
currentVersion: nonEnterpriseDefaultVersion,
138-
currentPath:
139-
'/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-branches',
140-
currentLanguage: 'en',
141-
}
142-
// This is needed because unit tests are weird. The page.render()
143-
// method is dependent on module global cache.
144-
// We need to fudge the `currentPath` so it appears to be different.
145-
context.currentPath += Math.random()
146-
await page.render(context)
147-
const $ = cheerio.load(page.intro)
148-
expect(
149-
$(
150-
'a[href="/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests"]'
151-
).length
152-
).toBe(0)
153-
expect(
154-
$(
155-
'a[href="/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests"]'
156-
).length
157-
).toBeGreaterThan(0)
158-
})
159-
160-
test('does not rewrite links that include deprecated enterprise release numbers', async () => {
161-
const page = await Page.init({
162-
relativePath:
163-
'admin/enterprise-management/updating-the-virtual-machine-and-physical-resources/migrating-from-github-enterprise-1110x-to-2123.md',
164-
basePath: path.join(__dirname, '../../content'),
165-
languageCode: 'en',
166-
})
167-
const context = {
168-
page: { version: `enterprise-server@${enterpriseServerReleases.latest}` },
169-
currentVersion: `enterprise-server@${enterpriseServerReleases.latest}`,
170-
currentPath: `/en/enterprise-server@${enterpriseServerReleases.latest}/admin/enterprise-management/migrating-from-github-enterprise-1110x-to-2123`,
171-
currentLanguage: 'en',
172-
}
173-
const rendered = await page.render(context)
174-
const $ = cheerio.load(rendered)
175-
expect(
176-
page.markdown.includes(
177-
'(/enterprise/11.10.340/admin/articles/upgrading-to-the-latest-release/)'
178-
)
179-
).toBe(true)
180-
expect(
181-
$(
182-
`a[href="/en/enterprise-server@${enterpriseServerReleases.latest}/11.10.340/admin/articles/upgrading-to-the-latest-release"]`
183-
).length
184-
).toBe(0)
185-
expect(
186-
$('a[href="/en/enterprise/11.10.340/admin/articles/upgrading-to-the-latest-release"]')
187-
.length
188-
).toBeGreaterThan(0)
189-
})
190-
191-
test('does not rewrite links to external redirects', async () => {
192-
const page = await Page.init(opts)
193-
page.markdown = `${page.markdown}\n\nSee [Capistrano](/capistrano).`
194-
const context = {
195-
page: { version: nonEnterpriseDefaultVersion },
196-
currentVersion: nonEnterpriseDefaultVersion,
197-
currentPath: `/en/${nonEnterpriseDefaultVersion}/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-branches`,
198-
currentLanguage: 'en',
199-
}
200-
const rendered = await page.render(context)
201-
const $ = cheerio.load(rendered)
202-
expect($('a[href="/capistrano"]').length).toBe(1)
203-
})
204-
71+
describe('page.render(context)', () => {
20572
// Most of our Liquid versioning tests are in https://github.com/docs/render-content,
20673
// But they don't have access to our currently supported versions, which we're testing here.
20774
// This test ensures that this works as expected: {% if enterpriseServerVersions contains currentVersion %}

0 commit comments

Comments
 (0)