Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify setup in happoPlaywright.init #9

Merged
merged 3 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
name: Run tests

on:
push:
branches: [main]
pull_request:
branches: [main]

jobs:
test:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: '20'
cache: 'yarn'

- name: Install dependencies
run: yarn install --frozen-lockfile

- name: Start dev server in the background
run: yarn dev &

- name: Install Playwright browsers
run: npx playwright install chromium

- name: Run tests
run: yarn test
env:
HAPPO_API_KEY: ${{ secrets.HAPPO_API_KEY }}
HAPPO_API_SECRET: ${{ secrets.HAPPO_API_SECRET }}
33 changes: 26 additions & 7 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,31 @@ const pathToBrowserBuild = require.resolve('happo-e2e/browser.build.js');

const controller = new Controller();

async function lazyLoadBrowserBundle(page) {
if (
await page.evaluate(() => typeof window.happoTakeDOMSnapshot === 'undefined')
) {
await page.addScriptTag({ path: pathToBrowserBuild });

// Add timeout check for happoTakeDOMSnapshot
try {
await page.waitForFunction(
() => typeof window.happoTakeDOMSnapshot !== 'undefined',
{ timeout: 10000 },
);
} catch (error) {
throw new Error('Timed out waiting for happoTakeDOMSnapshot to be defined');
}
}
}

module.exports = {
async init(contextOrPage) {
await contextOrPage.addInitScript({ path: pathToBrowserBuild });
await contextOrPage.addInitScript(`
window.addEventListener('load', () => {
window.happoTakeDOMSnapshot.init(window);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if we still needed to call this init function somewhere, but I found the function definition and discovered that it is now a noop, so removing this should be fine https://github.com/happo/happo-e2e/blob/5903e00c9dd2bb4d5a9cf9c2994181668ddfc634/takeDOMSnapshot.js#L347-L351

However, if it is possible for someone to have an updated version of happo-playwright but an old version of happo-e2e, they may run into a confusing situation. I'm not entirely sure what that experience would be like, but if we think it is likely that someone will run into this, then maybe we should make an adjustment to help them out.

});
`);
async init(pageOrContext) {
if (pageOrContext) {
console.warn(
'[HAPPO] You no longer need to pass a page or context to happoPlaywright.init()',
);
}
await controller.init();
},

Expand All @@ -35,6 +52,8 @@ module.exports = {
);
}

await lazyLoadBrowserBundle(page);

const elementHandle = handleOrLocator.elementHandle
? await handleOrLocator.elementHandle()
: handleOrLocator;
Expand Down
6 changes: 3 additions & 3 deletions tests/app.spec.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
const { test, expect } = require('@playwright/test');
const happoPlaywright = require('../');

test.beforeEach(async ({ context }) => {
await happoPlaywright.init(context);
test.beforeAll(async () => {
await happoPlaywright.init();
});

test.afterEach(async () => {
test.afterAll(async () => {
await happoPlaywright.finish();
});

Expand Down
Loading