Skip to content

Commit 8362602

Browse files
authored
do rendering end-to-end tests with a real server (github#26169)
* reinstate * start server manually * routing tests too * skip more * sleep more and fail if not 200 * use e2etest for content/ too * feedbacked
1 parent 84d1a6b commit 8362602

32 files changed

+360
-232
lines changed

.github/workflows/test.yml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,16 @@ jobs:
140140
NODE_ENV: test
141141
run: ./script/warm-before-tests.mjs
142142

143+
- name: Start production-like server in the background
144+
if: ${{ matrix.test-group == 'rendering' || matrix.test-group == 'routing' || matrix.test-group == 'content' }}
145+
env:
146+
NODE_ENV: test
147+
PORT: 4000
148+
run: |
149+
node server.mjs &
150+
sleep 3
151+
curl --retry-connrefused --retry 5 -I --fail http://localhost:4000/healthz
152+
143153
- name: Run tests
144154
env:
145155
DIFF_FILE: get_diff_files.txt

middleware/rate-limit.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import statsd from '../lib/statsd.js'
33

44
const EXPIRES_IN_AS_SECONDS = 60
55

6-
const MAX = process.env.RATE_LIMIT_MAX ? parseInt(process.env.RATE_LIMIT_MAX, 10) : 1000
6+
const MAX = process.env.RATE_LIMIT_MAX ? parseInt(process.env.RATE_LIMIT_MAX, 10) : 10000
77
if (isNaN(MAX)) {
88
throw new Error(`process.env.RATE_LIMIT_MAX (${process.env.RATE_LIMIT_MAX}) not a number`)
99
}

tests/content/featured-links.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import nock from 'nock'
77
import japaneseCharacters from 'japanese-characters'
88

99
import '../../lib/feature-flags.js'
10-
import { getDOM, getJSON } from '../helpers/supertest.js'
10+
import { getDOM, getJSON } from '../helpers/e2etest.js'
1111
import enterpriseServerReleases from '../../lib/enterprise-server-releases.js'
1212

1313
const __dirname = path.dirname(fileURLToPath(import.meta.url))

tests/content/search.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { dates, supported } from '../../lib/enterprise-server-releases.js'
44
import libLanguages from '../../lib/languages.js'
55
import { namePrefix } from '../../lib/search/config.js'
66
import lunrIndexNames from '../../script/search/lunr-get-index-names.js'
7-
import { get } from '../helpers/supertest.js'
7+
import { get } from '../helpers/e2etest.js'
88

99
const languageCodes = Object.keys(libLanguages)
1010

tests/content/webhooks.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { difference } from 'lodash-es'
2-
import { getJSON } from '../helpers/supertest.js'
2+
import { getJSON } from '../helpers/e2etest.js'
33
import { latest } from '../../lib/enterprise-server-releases.js'
44
import { allVersions } from '../../lib/all-versions.js'
55
import getWebhookPayloads from '../../lib/webhooks'

tests/helpers/caching-headers.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import { SURROGATE_ENUMS } from '../../middleware/set-fastly-surrogate-key.js'
2+
3+
export function checkCachingHeaders(res, defaultSurrogateKey = false, minMaxAge = 60 * 60) {
4+
expect(res.headers['set-cookie']).toBeUndefined()
5+
expect(res.headers['cache-control']).toContain('public')
6+
const maxAgeSeconds = parseInt(res.header['cache-control'].match(/max-age=(\d+)/)[1], 10)
7+
// Let's not be too specific in the tests, just as long as it's testing
8+
// that it's a reasonably large number of seconds.
9+
expect(maxAgeSeconds).toBeGreaterThanOrEqual(minMaxAge)
10+
// Because it doesn't have have a unique URL
11+
expect(res.headers['surrogate-key']).toBe(
12+
defaultSurrogateKey ? SURROGATE_ENUMS.DEFAULT : SURROGATE_ENUMS.MANUAL
13+
)
14+
}

tests/helpers/e2etest.js

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import cheerio from 'cheerio'
2+
import got from 'got'
3+
4+
export async function get(
5+
route,
6+
opts = {
7+
method: 'get',
8+
body: undefined,
9+
followRedirects: false,
10+
followAllRedirects: false,
11+
headers: {},
12+
}
13+
) {
14+
const method = opts.method || 'get'
15+
const fn = got[method]
16+
if (!fn || typeof fn !== 'function') throw new Error(`No method function for '${method}'`)
17+
const absURL = `http://localhost:4000${route}`
18+
const res = await fn(absURL, {
19+
body: opts.body,
20+
headers: opts.headers,
21+
retry: { limit: 0 },
22+
throwHttpErrors: false,
23+
followRedirect: opts.followAllRedirects || opts.followRedirects,
24+
})
25+
// follow all redirects, or just follow one
26+
if (opts.followAllRedirects && [301, 302].includes(res.status)) {
27+
// res = await get(res.headers.location, opts)
28+
throw new Error('A')
29+
} else if (opts.followRedirects && [301, 302].includes(res.status)) {
30+
// res = await get(res.headers.location)
31+
throw new Error('B')
32+
}
33+
34+
const text = res.body
35+
const status = res.statusCode
36+
const headers = res.headers
37+
return {
38+
text,
39+
status,
40+
statusCode: status, // Legacy
41+
headers,
42+
header: headers, // Legacy
43+
url: res.url,
44+
}
45+
}
46+
47+
export async function head(route, opts = { followRedirects: false }) {
48+
const res = await get(route, { method: 'head', followRedirects: opts.followRedirects })
49+
return res
50+
}
51+
52+
export function post(route, opts) {
53+
return get(route, Object.assign({}, opts, { method: 'post' }))
54+
}
55+
56+
export async function getDOM(
57+
route,
58+
{ headers, allow500s, allow404 } = { headers: undefined, allow500s: false, allow404: false }
59+
) {
60+
const res = await get(route, { followRedirects: true, headers })
61+
if (!allow500s && res.status >= 500) {
62+
throw new Error(`Server error (${res.status}) on ${route}`)
63+
}
64+
if (!allow404 && res.status === 404) {
65+
throw new Error(`Page not found on ${route}`)
66+
}
67+
const $ = cheerio.load(res.text || '', { xmlMode: true })
68+
$.res = Object.assign({}, res)
69+
return $
70+
}
71+
72+
// For use with the ?json query param
73+
// e.g. await getJSON('/en?json=breadcrumbs')
74+
export async function getJSON(route) {
75+
const res = await get(route, { followRedirects: true })
76+
if (res.status >= 500) {
77+
throw new Error(`Server error (${res.status}) on ${route}`)
78+
}
79+
if (res.status >= 400) {
80+
console.warn(`${res.status} on ${route} and the response might not be JSON`)
81+
}
82+
return JSON.parse(res.text)
83+
}

tests/rendering/breadcrumbs.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import { getDOM, getJSON } from '../helpers/supertest.js'
21
import { jest } from '@jest/globals'
32

3+
import { getDOM, getJSON } from '../helpers/e2etest.js'
4+
45
// TODO: Use `describeViaActionsOnly` instead. See tests/rendering/server.js
56
const describeInternalOnly =
67
process.env.GITHUB_REPOSITORY === 'github/docs-internal' ? describe : describe.skip

tests/rendering/curated-homepage-links.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getDOM } from '../helpers/supertest.js'
1+
import { getDOM } from '../helpers/e2etest.js'
22
import { jest } from '@jest/globals'
33

44
describe('curated homepage links', () => {

tests/rendering/favicons.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { expect } from '@jest/globals'
22

33
import { SURROGATE_ENUMS } from '../../middleware/set-fastly-surrogate-key.js'
4-
import { get } from '../helpers/supertest.js'
4+
import { get } from '../helpers/e2etest.js'
55

66
describe('favicon assets', () => {
77
it('should serve a valid and aggressively caching /favicon.ico', async () => {

0 commit comments

Comments
 (0)