diff --git a/.github/workflows/browser-test.yml b/.github/workflows/browser-test.yml index 1c37163775..8229cc43f4 100644 --- a/.github/workflows/browser-test.yml +++ b/.github/workflows/browser-test.yml @@ -27,10 +27,26 @@ concurrency: group: '${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }}' cancel-in-progress: true +env: + ELASTICSEARCH_URL: http://localhost:9200/ + jobs: build: runs-on: ${{ fromJSON('["ubuntu-latest", "ubuntu-20.04-xl"]')[github.repository == 'github/docs-internal'] }} steps: + - name: Install a local Elasticsearch for testing + # For the sake of saving time, only run this step if the test-group + # is one that will run tests against an Elasticsearch on localhost. + uses: getong/elasticsearch-action@95b501ab0c83dee0aac7c39b7cea3723bef14954 + with: + # Make sure this matches production and `sync-search-pr.yml` + elasticsearch version: '7.11.1' + host port: 9200 + container port: 9200 + host node port: 9300 + node port: 9300 + discovery type: 'single-node' + - name: Checkout uses: actions/checkout@dcd71f646680f2efd8db4afa5ad64fdcba30e748 with: @@ -62,5 +78,12 @@ jobs: - name: Run build script run: npm run build + - name: Index fixtures into the local Elasticsearch + run: npm run index-test-fixtures + + - name: Check that Elasticsearch is accessible + run: | + curl --fail --retry-connrefused --retry 5 -I ${{ env.ELASTICSEARCH_URL }} + - name: Run browser-test run: npm run browser-test diff --git a/components/Search.tsx b/components/Search.tsx index 85ff462bfe..13bd9e5aad 100644 --- a/components/Search.tsx +++ b/components/Search.tsx @@ -16,14 +16,16 @@ import { Link } from 'components/Link' import styles from './Search.module.scss' -// This is a temporary thing purely for the engineers of this project. -// When we are content that the new Elasticsearch-based middleware can -// wrap searches that match the old JSON format, but based on Elasticsearch -// behind the scene, we can change this component to always use -// /api/search/legacy. Then, when time allows we can change this component -// to use the new JSON format (/api/search/v1) and change the code to -// use that instead. -const USE_LEGACY_SEARCH = JSON.parse(process.env.NEXT_PUBLIC_USE_LEGACY_SEARCH || 'false') +// The search endpoint used prior to using /api/search/legacy was +// just /search, which used middleware/search.js. We are leaving that +// middleware in tact to allow folks that previously used the /search +// endpoint to continue doing so. But, we changed the endpoint used by +// the search input on docs.github.com to use the new /api/search/legacy +// endpoint. +// Eventually, we will deprecate the /search and /api/search/legacy +// endpoints and use the /api/search/v1 endpoint, which has +// a different response JSON format. +const SEARCH_API_ENDPOINT = '/api/search/legacy' type SearchResult = { url: string @@ -67,7 +69,7 @@ export function Search({ : 'en' const fetchURL = query - ? `/${USE_LEGACY_SEARCH ? 'api/search/legacy' : 'search'}?${new URLSearchParams({ + ? `${SEARCH_API_ENDPOINT}?${new URLSearchParams({ language, version, query, diff --git a/tests/browser/browser.js b/tests/browser/browser.js index 3c7c556029..1e04bfd9ae 100644 --- a/tests/browser/browser.js +++ b/tests/browser/browser.js @@ -13,6 +13,8 @@ describe('homepage', () => { }) }) +// Note, we can only test Elasticsearch searches on things we have indexed +// in the fixtures. See the contents of /tests/content/fixtures/search-indexes/ describe('browser search', () => { jest.setTimeout(60 * 1000) @@ -20,10 +22,10 @@ describe('browser search', () => { await page.goto('http://localhost:4000/en/actions') await page.click('[data-testid=mobile-menu-button]') await page.click('[data-testid=mobile-header] [data-testid=site-search-input]') - await page.type('[data-testid=mobile-header] [data-testid=site-search-input]', 'workflows') + await page.type('[data-testid=mobile-header] [data-testid=site-search-input]', 'foo') await page.waitForSelector('[data-testid=search-results]') const hits = await page.$$('[data-testid=search-result]') - expect(hits.length).toBeGreaterThan(5) + expect(hits.length).toBeGreaterThan(3) }) it('works on desktop landing pages', async () => { @@ -31,10 +33,10 @@ describe('browser search', () => { await page.setViewport({ width: 1024, height: 768 }) await page.goto('http://localhost:4000/en/actions') await page.click('[data-testid=desktop-header] [data-testid=site-search-input]') - await page.type('[data-testid=desktop-header] [data-testid=site-search-input]', 'workflows') + await page.type('[data-testid=desktop-header] [data-testid=site-search-input]', 'foo') await page.waitForSelector('[data-testid=search-results]') const hits = await page.$$('[data-testid=search-result]') - expect(hits.length).toBeGreaterThan(5) + expect(hits.length).toBeGreaterThan(3) await page.setViewport(initialViewport) }) // 404 page is statically generated with next, so search is not available, but may possibly be brought back @@ -48,7 +50,8 @@ describe('browser search', () => { expect(hits.length).toBeGreaterThan(5) }) - it('sends the correct data to search for Enterprise Server', async () => { + // Elasticsearch fixtures only work for dotco and GHAE + it.skip('sends the correct data to search for Enterprise Server', async () => { expect.assertions(2) const newPage = await browser.newPage() @@ -58,7 +61,10 @@ describe('browser search', () => { await newPage.setRequestInterception(true) newPage.on('request', (interceptedRequest) => { - if (interceptedRequest.method() === 'GET' && /search\?/i.test(interceptedRequest.url())) { + if ( + interceptedRequest.method() === 'GET' && + /api\/search\/legacy\?/i.test(interceptedRequest.url()) + ) { const { searchParams } = new URL(interceptedRequest.url()) expect(searchParams.get('version')).toBe(oldestSupported) expect(searchParams.get('language')).toBe('en') @@ -75,17 +81,20 @@ describe('browser search', () => { await newPage.waitForSelector('[data-testid=search-result]') }) - it('sends the correct data to search for GHEC', async () => { + it('sends the correct data to search for dotcom', async () => { expect.assertions(2) const newPage = await browser.newPage() - await newPage.goto('http://localhost:4000/en/enterprise-cloud@latest/admin/overview') + await newPage.goto('http://localhost:4000/en') await newPage.setRequestInterception(true) newPage.on('request', (interceptedRequest) => { - if (interceptedRequest.method() === 'GET' && /search\?/i.test(interceptedRequest.url())) { + if ( + interceptedRequest.method() === 'GET' && + /api\/search\/legacy\?/i.test(interceptedRequest.url()) + ) { const { searchParams } = new URL(interceptedRequest.url()) - expect(searchParams.get('version')).toBe('ghec') + expect(searchParams.get('version')).toBe('dotcom') expect(searchParams.get('language')).toBe('en') } interceptedRequest.continue() @@ -96,7 +105,7 @@ describe('browser search', () => { '[data-testid=mobile-header] [data-testid=site-search-input]' ) await searchInput.click() - await searchInput.type('test') + await searchInput.type('foo') await newPage.waitForSelector('[data-testid=search-result]') }) @@ -108,7 +117,10 @@ describe('browser search', () => { await newPage.setRequestInterception(true) newPage.on('request', (interceptedRequest) => { - if (interceptedRequest.method() === 'GET' && /search\?/i.test(interceptedRequest.url())) { + if ( + interceptedRequest.method() === 'GET' && + /api\/search\/legacy\?/i.test(interceptedRequest.url()) + ) { const { searchParams } = new URL(interceptedRequest.url()) expect(searchParams.get('version')).toBe('ghae') expect(searchParams.get('language')).toBe('en') @@ -121,7 +133,7 @@ describe('browser search', () => { '[data-testid=mobile-header] [data-testid=site-search-input]' ) await searchInput.click() - await searchInput.type('test') + await searchInput.type('silly') await newPage.waitForSelector('[data-testid=search-result]') }) })