Skip to content

Commit

Permalink
fix: no-sw error shows for subdomain requests (#491)
Browse files Browse the repository at this point in the history
* test: enable testing of pages with no service worker loaded

* test: add no-sw tests that repro #272

* fix: no-sw error shows for subdomain requests

* test: no-service-worker tests target a test file & run in CI
  • Loading branch information
SgtPooki authored Nov 22, 2024
1 parent d50f5bc commit c16688e
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 6 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,4 @@ playwright-report
.coverage
test-e2e/fixtures/data/test-repo
test-e2e/fixtures/data/gateway-conformance-fixtures
test-results
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
"test:iso": "aegir test -f dist-tsc/test/**/*.spec.js",
"test:browsers": "playwright test -c playwright.config.js",
"test:chrome": "playwright test -c playwright.config.js --project chromium",
"test:firefox": "playwright test -c playwright.config.js --project firefox",
"test:no-sw": "playwright test -c playwright.config.js --project no-service-worker",
"test:firefox": "playwright test -c playwright.config.js --project firefox --project no-service-worker",
"test:deployed": "playwright test -c playwright.config.js --project deployed",
"test:inbrowser-dev": "cross-env BASE_URL='https://inbrowser.dev' playwright test -c playwright.config.js --project deployed",
"test:inbrowser-prod": "cross-env BASE_URL='https://inbrowser.link' playwright test -c playwright.config.js --project deployed",
Expand Down
26 changes: 26 additions & 0 deletions playwright.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,32 @@ export default defineConfig({
...devices['Desktop Firefox'],
baseURL: process.env.BASE_URL
}
},
{
/**
* Test the site with service workers disabled. You need to `import {testNoServiceWorker as test, expect} from './fixtures/config-test-fixtures.js'` to use this project.
* Anything needing a service worker will be skipped when this project is ran.
*/
name: 'no-service-worker',
testMatch: /test-e2e\/no-service-worker\.test\.ts/,
use: {
...devices['Desktop Firefox'],
contextOptions: {
serviceWorkers: 'block'
},
launchOptions: {
firefoxUserPrefs: {
'dom.serviceWorkers.enabled': false
}
},
beforeEach: async ({ page }) => {
await page.addInitScript(() => {
Object.defineProperty(navigator, 'serviceWorker', {
get: () => undefined
})
})
}
}
}
],
// TODO: disable webservers when testing `deployed` project
Expand Down
1 change: 1 addition & 0 deletions src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ if ('serviceWorker' in navigator) {

const routes: Route[] = [
{ default: true, component: ErrorPage ?? LazyHelperUi },
{ shouldRender: async () => renderChecks.shouldRenderNoServiceWorkerError(), component: LazyServiceWorkerErrorPage },
{ shouldRender: async () => renderChecks.shouldRenderRedirectsInterstitial(), component: LazyInterstitial },
{ path: '#/ipfs-sw-config', shouldRender: async () => renderChecks.shouldRenderConfigPage(), component: LazyConfig },
{
Expand Down
4 changes: 4 additions & 0 deletions src/lib/routing-render-checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,7 @@ export function shouldRenderRedirectsInterstitial (): boolean {
const heliaSw = url.searchParams.get('helia-sw')
return heliaSw != null
}

export function shouldRenderNoServiceWorkerError (): boolean {
return !('serviceWorker' in navigator)
}
2 changes: 1 addition & 1 deletion src/pages/errors/no-service-worker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default function NoServiceWorkerErrorPage (): React.JSX.Element {
return (
<>
<Header />
<main className='pa4-l bg-red-muted mw7 mb5 center pa4'>
<main className='pa4-l bg-red-muted mw7 mb5 center pa4 e2e-no-service-worker-error'>
<h1>Service Worker Error</h1>
<p>
This page requires a service worker to be available. Please ensure that
Expand Down
34 changes: 31 additions & 3 deletions test-e2e/fixtures/config-test-fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import { test as base, type Page } from '@playwright/test'
import { setConfig, setSubdomainConfig } from './set-sw-config.js'
import { waitForServiceWorker } from './wait-for-service-worker.js'

function isNoServiceWorkerProject <T extends typeof base = typeof base> (test: T): boolean {
return test.info().project.name === 'no-service-worker'
}

const rootDomain = async ({ baseURL }, use): Promise<void> => {
const url = new URL(baseURL)
await use(url.host)
Expand All @@ -13,13 +17,20 @@ const baseURLProtocol = async ({ baseURL }, use): Promise<void> => {

export const test = base.extend<{ rootDomain: string, baseURL: string, protocol: string }>({
rootDomain: [rootDomain, { scope: 'test' }],
protocol: [baseURLProtocol, { scope: 'test' }]
protocol: [baseURLProtocol, { scope: 'test' }],
page: async ({ page }, use) => {
if (isNoServiceWorkerProject(test)) {
test.skip()
return
}
await use(page)
}
})

/**
* You should use this fixture instead of the `test` fixture from `@playwright/test` when testing path routing via the service worker.
*/
export const testPathRouting = base.extend<{ rootDomain: string, baseURL: string, protocol: string }>({
export const testPathRouting = test.extend<{ rootDomain: string, baseURL: string, protocol: string }>({
rootDomain: [rootDomain, { scope: 'test' }],
protocol: [baseURLProtocol, { scope: 'test' }],
page: async ({ page, rootDomain }, use) => {
Expand Down Expand Up @@ -63,7 +74,7 @@ export const testPathRouting = base.extend<{ rootDomain: string, baseURL: string
* })
* ```
*/
export const testSubdomainRouting = base.extend<{ rootDomain: string, baseURL: string, protocol: string }>({
export const testSubdomainRouting = test.extend<{ rootDomain: string, baseURL: string, protocol: string }>({
rootDomain: [rootDomain, { scope: 'test' }],
protocol: [baseURLProtocol, { scope: 'test' }],
page: async ({ page, baseURL }, use) => {
Expand Down Expand Up @@ -108,4 +119,21 @@ export const testSubdomainRouting = base.extend<{ rootDomain: string, baseURL: s
}
})

/**
* A fixture that skips tests that require the service worker. This is needed in order to test handling of requests where the service worker is not present.
*
* @see https://github.com/ipfs/service-worker-gateway/issues/272
*/
export const testNoServiceWorker = base.extend<{ rootDomain: string, baseURL: string, protocol: string }>({
rootDomain: [rootDomain, { scope: 'test' }],
protocol: [baseURLProtocol, { scope: 'test' }],
page: async ({ page }, use) => {
if (!isNoServiceWorkerProject(testNoServiceWorker)) {
testNoServiceWorker.skip()
return
}
await use(page)
}
})

export { expect } from '@playwright/test'
2 changes: 2 additions & 0 deletions test-e2e/fixtures/locators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export const getConfigGatewaysInput: GetLocator = (page) => page.locator('.e2e-c
export const getConfigRoutersInput: GetLocator = (page) => page.locator('.e2e-config-page-input-routers')
export const getConfigAutoReloadInput: GetLocator = (page) => page.locator('.e2e-config-page-input-autoreload')

export const getNoServiceWorkerError: GetLocator = (page) => page.locator('.e2e-no-service-worker-error')

/**
* Iframe page parts
*/
Expand Down
2 changes: 1 addition & 1 deletion test-e2e/layout.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { test, expect } from '@playwright/test'
import { test, expect } from './fixtures/config-test-fixtures.js'
import { getConfigButton, getConfigPage, getConfigPageSaveButton, getConfigPageInput, getHeader, getHeaderTitle } from './fixtures/locators.js'

test.describe('smoketests', () => {
Expand Down
16 changes: 16 additions & 0 deletions test-e2e/no-service-worker.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { testNoServiceWorker as test, expect } from './fixtures/config-test-fixtures.js'
import { getNoServiceWorkerError } from './fixtures/locators.js'

test.describe('no-service-worker', () => {
test('Error renders on landing page', async ({ page }) => {
await page.goto('/')

await expect(getNoServiceWorkerError(page)).toBeVisible()
})

test('Error renders on subdomain page', async ({ page, rootDomain, protocol }) => {
await page.goto(`${protocol ?? 'http'}//bafkqablimvwgy3y.ipfs.${rootDomain}`, { waitUntil: 'networkidle' })

await expect(getNoServiceWorkerError(page)).toBeVisible()
})
})

0 comments on commit c16688e

Please sign in to comment.