diff --git a/.github/workflows/local-dev.yml b/.github/workflows/local-dev.yml index e54d1375d9..6653b5f41b 100644 --- a/.github/workflows/local-dev.yml +++ b/.github/workflows/local-dev.yml @@ -1,9 +1,7 @@ name: Local development -# **What it does**: Can you start the local server like a writer would do? -# **Why we have it**: Our CI is often heavily geared on testing in "production" -# that historically we've been known to break local -# development sometimes. +# **What it does**: Basic smoke test to ensure local dev server starts and serves content +# **Why we have it**: Catch catastrophic "npm start is completely broken" scenarios # **Who does it impact**: Engineers, Contributors. on: @@ -28,76 +26,25 @@ jobs: with: token: ${{ secrets.DOCS_BOT_PAT_BASE }} - # Note that we don't check out docs-early-access, Elasticsearch, - # or any remote translations. Nothing fancy here! - - name: Disable Next.js telemetry run: npx next telemetry disable - - name: Install headless browser - run: npx playwright install --no-shell - - # The Playwright test, with the env vars we set here, takes care of - # starting a server and shutting it down when it's done. - # That's why it's important this step comes before the `npm start &` - # step below. - - name: Run Playwright tests - env: - # This is what local dev contributors are expected to do. - PLAYWRIGHT_START_SERVER_COMMAND: 'npm start' - # This is so that timeouts aren't retried, which can lead to - # tests not exiting at the end with a non-zero. Otherwise, - # by default failures are marked as "flaky" instead of "failed". - PLAYWRIGHT_RETRIES: 0 - TEST_EARLY_ACCESS: ${{ github.repository == 'github/docs-internal' }} - # workaround for https://github.com/nodejs/node/issues/59364 as of 22.18.0 - NODE_OPTIONS: '--no-experimental-strip-types --max-old-space-size=8192' - run: npm run playwright-test -- playwright-local-dev - - - name: Start server in the background - run: npm start > /tmp/stdout.log 2> /tmp/stderr.log & - - - name: View the home page + - name: Start server and basic smoke test run: | - echo "Going to sleep a little to wait for the server to start" - sleep 15 - curl --fail --retry-connrefused --retry 5 http://localhost:4000/ + # Start server in background + npm start > /tmp/stdout.log 2> /tmp/stderr.log & + SERVER_PID=$! - - name: Run basic tests - run: npm run test-local-dev - - - if: ${{ failure() }} - name: Debug server outputs on errors - run: | - echo "____STDOUT____" - cat /tmp/stdout.log - echo "____STDERR____" - cat /tmp/stderr.log - - - name: Pre-commit hooks should prevent bad Markdown edits - run: | - set -e - - # This test assumes this one file always exists - ls content/get-started/start-your-journey/hello-world.md - - # Not sure if it matters but we're in a detached HEAD state - # after the actions/checkout action. - git checkout -b my-new-branch - # Also, do this so you don't get errors from git about this - # not being set up before your first commit attempt - git config user.name github-actions - git config user.email github-actions@github.com - - # To know what will fail the markdown lint, see src/content-linter/style/github-docs.js - # Add some NOT valid Markdown to it - # In this case an internal link with a hardcode /en/ prefix. - echo "This *is** not valid [Markdown](/en/foo)" >> content/get-started/start-your-journey/hello-world.md - git commit -a -m "this should fail" - exit_code=$? - if [ $exit_code != 0 ]; then - echo "That SHOULD have failed, but it DIDN'T" - exit 1 + # Wait for server to be ready and test homepage + if curl --fail --retry-connrefused --retry 10 --retry-delay 2 http://localhost:4000/; then + echo "✅ Local dev server started successfully and serves homepage" + kill $SERVER_PID 2>/dev/null || true else - echo "As expected, it failed :)" + echo "❌ Local dev server failed to start or serve content" + echo "____STDOUT____" + cat /tmp/stdout.log + echo "____STDERR____" + cat /tmp/stderr.log + kill $SERVER_PID 2>/dev/null || true + exit 1 fi diff --git a/package.json b/package.json index b53e8f248c..b08751c59f 100644 --- a/package.json +++ b/package.json @@ -91,7 +91,6 @@ "sync-secret-scanning": "tsx src/secret-scanning/scripts/sync.ts", "sync-webhooks": "npx tsx src/rest/scripts/update-files.ts -o webhooks", "test": "vitest", - "test-local-dev": "tsx src/workflows/test-local-dev.ts", "test-moved-content": "tsx src/content-render/scripts/test-moved-content.ts", "tsc": "tsc --noEmit", "unallowed-contributions": "tsx src/workflows/unallowed-contributions.ts", diff --git a/src/fixtures/tests/playwright-local-dev.spec.ts b/src/fixtures/tests/playwright-local-dev.spec.ts deleted file mode 100644 index 8f85e6d8d6..0000000000 --- a/src/fixtures/tests/playwright-local-dev.spec.ts +++ /dev/null @@ -1,65 +0,0 @@ -/** - * These tests assume you have started the local dev server as a contributor - * would. It does *not* use fixture data. It uses real English content - * as seen in `main` or in the current branch. Therefore be careful - * with what you can expect to find. Stick to known and stable content. - * - * It's always a risk that the content changes and can break tests - * that exist to test the *code*. But these tests are ultimately there to - * do what a human would do which is: Start the server, then open the - * browser, then click around, then search, etc. - * - */ - -import { test, expect } from '@playwright/test' -import { dismissCTAPopover, turnOffExperimentsInPage } from '../helpers/turn-off-experiments' - -const TEST_EARLY_ACCESS = Boolean(JSON.parse(process.env.TEST_EARLY_ACCESS || 'false')) - -test('view home page', async ({ page }) => { - await page.goto('/') - await turnOffExperimentsInPage(page) - await dismissCTAPopover(page) - - await expect(page).toHaveTitle(/GitHub Docs/) -}) - -test('click "Get started" from home page', async ({ page }) => { - await page.goto('/') - await turnOffExperimentsInPage(page) - await dismissCTAPopover(page) - - await page.getByRole('link', { name: 'Get started' }).click() - await expect(page).toHaveTitle(/Get started with GitHub/) - await expect(page).toHaveURL(/\/en\/get-started/) -}) - -test('search "foo" and get results', async ({ page }) => { - await page.goto('/') - await turnOffExperimentsInPage(page) - await dismissCTAPopover(page) - - await page.locator('[data-testid="search"]:visible').click() - await page.getByTestId('overlay-search-input').fill('foo') - // Wait for search results to load - await page.waitForTimeout(1000) - // Click "View more results" to get to the search page - await page.getByText('View more results').click() - await expect(page.getByRole('heading', { name: /\d+ Search results for "foo"/ })).toBeVisible() -}) - -test('view the early-access links page', async ({ page }) => { - if (!TEST_EARLY_ACCESS) return - - await page.goto('/early-access') - await turnOffExperimentsInPage(page) - await dismissCTAPopover(page) - - await expect(page).toHaveURL(/\/en\/early-access/) - await page.getByRole('heading', { name: 'Early Access documentation', level: 1 }).click() - const links = await page.$$eval( - '#article-contents ul li a', - (elements: HTMLAnchorElement[]) => elements, - ) - expect(links.length).toBeGreaterThan(0) -}) diff --git a/src/fixtures/tests/playwright-rendering.spec.ts b/src/fixtures/tests/playwright-rendering.spec.ts index e7d698761e..42f24f2012 100644 --- a/src/fixtures/tests/playwright-rendering.spec.ts +++ b/src/fixtures/tests/playwright-rendering.spec.ts @@ -199,6 +199,14 @@ test('search from enterprise-cloud and filter by top-level Fooing', async ({ pag // for improvement! }) +test('404 page renders correctly', async ({ page }) => { + const response = await page.goto('/this-definitely-does-not-exist') + expect(response?.status()).toBe(404) + + // Check that the 404 page content is rendered + await expect(page.getByText(/It looks like this page doesn't exist/)).toBeVisible() +}) + test.describe('platform picker', () => { test('switch operating systems', async ({ page }) => { await page.goto('/get-started/liquid/platform-specific') diff --git a/src/workflows/test-local-dev.ts b/src/workflows/test-local-dev.ts deleted file mode 100755 index aff7dad538..0000000000 --- a/src/workflows/test-local-dev.ts +++ /dev/null @@ -1,177 +0,0 @@ -import assert from 'node:assert/strict' -import fs from 'fs' - -import cheerio from 'cheerio' - -/** - * A very basic script that tests the local dev server. - * - * We use this in CI to make sure the local development server works. - * There are certain things that only work and happen when in - * local dev, that don't make sense to test in regular end-to-end tests - * such as `vitest` rendering. - * - * For engineers to test this locally do the following: - * - * 1. Start `npm run dev` in one terminal - * 2. Run `src/workflows/test-local-dev.ts` in another terminal - * - */ - -main() - -async function get(path: string, options?: Record) { - // By default, fetch() will follow redirects. - const t0 = new Date() - const response = await fetch(makeURL(path), options) - const took = new Date().getTime() - t0.getTime() - console.log(`GET ${path} => ${response.status} (${took}ms)`) - - // Convert fetch response to have similar interface as got response - const body = await response.text() - return { - statusCode: response.status, - body: body, - } -} - -function makeURL(path: string) { - return `http://localhost:4000${path}` -} - -async function main() { - // Edit a page's content and expect to see that change when viewed - await testEditingPage() - - // Only in local dev is the `?json=...` query string working - await testJSONParameters() - - // In local development, it depends on proxying the search to prod - // because if you haven't set up ELASTICSEARCH_URL. - await testSiteSearch() - - await testViewingPages() - - // Next.js uses just-in-time compilation to compile pages on demand. - // But once the server is up it should *not crash* to request these things. - await testNextJsSpecialURLs() -} - -async function testEditingPage() { - const string = `Today's date is ${new Date().toString()}` - const filePath = 'content/get-started/start-your-journey/hello-world.md' - const content = fs.readFileSync(filePath, 'utf-8') - try { - fs.appendFileSync(filePath, string, 'utf-8') - - const res = await get('/get-started/start-your-journey/hello-world') - if (!res.body.includes(string)) { - throw new Error(`Couldn't find the string '${string}' in the response body`) - } - } finally { - fs.writeFileSync(filePath, content, 'utf-8') - } -} - -async function testJSONParameters() { - // currentVersion should be free-pro-team@latest - { - const res = await get('/get-started/start-your-journey/hello-world?json=currentVersion') - const info = JSON.parse(res.body) - assert(info === 'free-pro-team@latest') - } - - // currentVersion should be free-pro-team@latest - { - const res = await get('/enterprise-server@latest/admin?json=currentVersion') - const info = JSON.parse(res.body) - assert(/enterprise-server@\d/.test(info)) - } - - // currentProduct (home page) - { - const res = await get('/en?json=currentProduct') - const currentProduct = JSON.parse(res.body) - assert(currentProduct === 'homepage') - } - - // currentProduct (actions) - { - const res = await get('/en/actions?json=currentProduct') - const currentProduct = JSON.parse(res.body) - assert(currentProduct === 'actions') - } - - // page - { - const res = await get('/en?json=page') - const info = JSON.parse(res.body) - assert(info.title === 'GitHub.com Help Documentation') - } - - // Just ?json - { - const res = await get('/en?json') - const info = JSON.parse(res.body) - assert(info.message) - assert(info.keys && Array.isArray(info.keys)) - } - - // Featured links - { - const res = await get('/en?json=featuredLinks.gettingStarted') - const links = JSON.parse(res.body) - assert(links && Array.isArray(links)) - assert(links[0].href) - assert(links[0].page) - } -} - -async function testSiteSearch() { - // Find something on free-pro-team@latest - { - const res = await get('/en/search?query=github') - const $ = cheerio.load(res.body) - // The [\d,]+ is because we use thousands separators in the number - assert(/[\d,]+ Search results for "github"/.test($('h1').text())) - assert($('[data-testid="search-result"]').length > 0) - } - // Find 0 things on enterprise-cloud@latest - { - const res = await get('/en/enterprise-cloud@latest/search?query=gobligook') - const $ = cheerio.load(res.body) - assert(/0 Search results for "gobligook"/.test($('h1').text())) - assert($('[data-testid="search-result"]').length === 0) - } - // Using the search API - { - const res = await get('/api/search?query=github') - const results = JSON.parse(res.body) - assert(results.meta) - assert(results.hits) - } -} - -async function testViewingPages() { - // Getting a 404 page with an /en/ prefix should be a 404 HTML page - const res = await get('/en/never/heard/of', { - throwHttpErrors: false, - }) - assert(res.statusCode === 404) - // console.log(res.body) - const $ = cheerio.load(res.body) - assert(/It looks like this page doesn't exist./.test($('article').text())) -} - -async function testNextJsSpecialURLs() { - // _next/webpack-hmr - { - const res = await get('/_next/webpack-hmr') - assert(res.statusCode === 200) - } - // _next/static/webpack/HASH.webpack.hot-update.json - { - const res = await get('/_next/static/webpack/deadbeefdeadbeef.webpack.hot-update.json') - assert(res.statusCode === 200) - } -}