From 2157d69ba9d0a8dd2a95509fa959f91fdfce7d0b Mon Sep 17 00:00:00 2001 From: Darius Jahandarie Date: Wed, 1 Jan 2025 07:02:48 +0900 Subject: [PATCH] Speed up playwright (pt2: full parallelization) (#1720) * Fully separate all popup tests * Shard playwright CI --- .github/workflows/playwright.yml | 113 +++++++++++++++++--- .github/workflows/playwright_comment.yml | 18 +--- playwright.config.js | 5 +- test/playwright/visual.spec.js | 128 +++++++++++------------ 4 files changed, 164 insertions(+), 100 deletions(-) diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index 5c2f1db42..357178f1c 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -6,15 +6,15 @@ on: permissions: contents: read jobs: - playwright: + playwright-tests: timeout-minutes: 60 runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + shardIndex: [1, 2, 3, 4, 5] + shardTotal: [5] steps: - - name: Harden Runner - uses: step-security/harden-runner@v2 - with: - egress-policy: audit - - name: Remove all fonts run: rm -rf /usr/share/fonts @@ -66,24 +66,28 @@ jobs: - name: "[PR] Generate new screenshots & compare against master" id: playwright run: | - npx playwright test 2>&1 | tee ./playwright-output || true + npx playwright test --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} 2>&1 | tee ./playwright-output || true continue-on-error: true if: github.event_name == 'pull_request' && steps.master-screenshots.outcome != 'failure' - name: "[Push] Generate new authoritative screenshots for master" id: playwright-master - run: npx playwright test -u + run: npx playwright test --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }} -u if: github.event_name == 'push' - uses: actions/upload-artifact@v4 with: - name: playwright-screenshots + name: playwright-screenshots-${{ matrix.shardIndex }} path: test/playwright/__screenshots__/ + retention-days: 1 - - uses: actions/upload-artifact@v4 + - name: Upload blob report to GitHub Actions Artifacts + if: ${{ !cancelled() }} + uses: actions/upload-artifact@v4 with: - name: playwright-report - path: playwright-report/ + name: blob-report-${{ matrix.shardIndex }} + path: blob-report + retention-days: 1 - name: "[PR] Store steps.master-screenshots.outcome in a file" run: echo ${{ steps.master-screenshots.outcome }} > ./master-screenshots-outcome @@ -91,18 +95,97 @@ jobs: - uses: actions/upload-artifact@v4 with: - name: master-screenshots-outcome + name: master-screenshots-outcome-${{ matrix.shardIndex }} path: master-screenshots-outcome if: github.event_name == 'pull_request' - uses: actions/upload-artifact@v4 with: - name: playwright-output + name: playwright-output-${{ matrix.shardIndex }} path: playwright-output + retention-days: 1 if: github.event_name == 'pull_request' + playwright: + timeout-minutes: 60 + runs-on: ubuntu-latest + # Merge reports after playwright-tests, even if some shards have failed + if: ${{ !cancelled() }} + needs: [playwright-tests] + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/setup + + ### playwright-report + - name: Download blob reports from GitHub Actions Artifacts + uses: actions/download-artifact@v4 + with: + path: all-blob-reports + pattern: blob-report-* + merge-multiple: true + + - name: Merge into HTML Report + run: npx playwright merge-reports --reporter html ./all-blob-reports + + - name: Merge into JSON Report + run: npx playwright merge-reports --reporter json ./all-blob-reports > playwright-results.json + + - uses: actions/upload-artifact@v4 + with: + name: playwright-report + path: playwright-report/ + - uses: actions/upload-artifact@v4 with: name: playwright-results-json path: playwright-results.json - if: github.event_name == 'pull_request' + + ### playwright-screenshots + - name: Download & merge screenshots from GitHub Actions Artifacts + uses: actions/download-artifact@v4 + with: + path: test/playwright/__screenshots__ + pattern: playwright-screenshots-* + merge-multiple: true + + - uses: actions/upload-artifact@v4 + with: + name: playwright-screenshots + path: test/playwright/__screenshots__/ + + ### master-screenshots-outcome + - name: Download master-screenshots-outcome files + uses: actions/download-artifact@v4 + with: + path: master-screenshots-outcomes + pattern: master-screenshots-outcome-* + merge-multiple: true + + - name: Check if any shard failed to download master screenshots + run: | + if grep -q "failure" master-screenshots-outcomes/*; then + echo "failure" > master-screenshots-outcome + else + echo "success" > master-screenshots-outcome + fi + + - uses: actions/upload-artifact@v4 + with: + name: master-screenshots-outcome + path: master-screenshots-outcome + + ### playwright-output + - name: Download playwright-output files + uses: actions/download-artifact@v4 + with: + path: playwright-outputs + pattern: playwright-output-* + merge-multiple: true + + - name: Merge playwright output files + run: cat playwright-outputs/* > playwright-output + + - uses: actions/upload-artifact@v4 + with: + name: playwright-output + path: playwright-output diff --git a/.github/workflows/playwright_comment.yml b/.github/workflows/playwright_comment.yml index 9a9938e24..8a32d4ad7 100644 --- a/.github/workflows/playwright_comment.yml +++ b/.github/workflows/playwright_comment.yml @@ -36,14 +36,6 @@ jobs: run_id: ${{ github.event.workflow_run.id }} name: master-screenshots-outcome - - name: Grab playwright-results-json from PR run - uses: dawidd6/action-download-artifact@80620a5d27ce0ae443b965134db88467fc607b43 # v7 - continue-on-error: true - with: - github_token: ${{ secrets.GITHUB_TOKEN }} - run_id: ${{ github.event.workflow_run.id }} - name: playwright-results-json - - name: Dry-run grab playwright-report from PR run so we have its ID uses: dawidd6/action-download-artifact@80620a5d27ce0ae443b965134db88467fc607b43 # v7 id: playwright-report @@ -62,12 +54,6 @@ jobs: ID=$(echo "$ARTIFACTS_JSON" | jq -r '.[0].id'); echo "id=$ID" >> "$GITHUB_OUTPUT" - - name: Generate summary from playwright-results.json (expected to fail to comment) - id: playwright-summary - uses: daun/playwright-report-summary@v3 - with: - report-file: playwright-results.json - - name: Load artifacts into environment variables id: playwright run: | @@ -79,8 +65,8 @@ jobs: echo "MASTER_SCREENSHOTS_OUTCOME<<$EOF"; cat ./master-screenshots-outcome; echo "$EOF"; - echo "FAILED=$(grep -c '^ *[0-9] failed$' ./playwright-output)"; - echo "FLAKY=$(grep -c '^ *[0-9] flaky$' ./playwright-output)" + echo "FAILED=$(jq -r '.stats.unexpected' playwright-results.json)"; + echo "FLAKY=$(jq -r '.stats.flaky' playwright-results.json)"; } >> "$GITHUB_OUTPUT" # this is required because github.event.workflow_run.pull_requests is not available for PRs from forks diff --git a/playwright.config.js b/playwright.config.js index 2e54a19cc..e415bac9e 100644 --- a/playwright.config.js +++ b/playwright.config.js @@ -47,10 +47,7 @@ export default defineConfig({ /* Opt out of parallel tests on CI. */ workers: process.env.CI ? 1 : void 0, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: [ - ['html'], - ['json', {outputFile: 'playwright-results.json'}], - ], + reporter: process.env.CI ? 'blob' : 'html', /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */ diff --git a/test/playwright/visual.spec.js b/test/playwright/visual.spec.js index 3812fc993..87103eb87 100644 --- a/test/playwright/visual.spec.js +++ b/test/playwright/visual.spec.js @@ -15,6 +15,7 @@ * along with this program. If not, see . */ +import {readFileSync} from 'fs'; import path from 'path'; import {pathToFileURL} from 'url'; import {createDictionaryArchiveData} from '../../dev/dictionary-archive-util.js'; @@ -95,71 +96,68 @@ test('settings', async ({page, extensionId}) => { }); }); -test('popup', async ({page, extensionId}) => { - // Open settings - console.log('Open settings'); - await page.goto(`chrome-extension://${extensionId}/settings.html`); - - await expect(page.locator('id=dictionaries')).toBeVisible(); - - // Load in test dictionary - const dictionary = await createDictionaryArchiveData(path.join(root, 'test/data/dictionaries/valid-dictionary1'), 'valid-dictionary1'); - await page.locator('input[id="dictionary-import-file-input"]').setInputFiles({ - name: 'valid-dictionary1.zip', - mimeType: 'application/x-zip', - buffer: Buffer.from(dictionary), +test.describe('popup', () => { + test.beforeEach(async ({page, extensionId}) => { + // Open settings + console.log('Open settings'); + await page.goto(`chrome-extension://${extensionId}/settings.html`); + + await expect(page.locator('id=dictionaries')).toBeVisible(); + + // Load in test dictionary + const dictionary = await createDictionaryArchiveData(path.join(root, 'test/data/dictionaries/valid-dictionary1'), 'valid-dictionary1'); + await page.locator('input[id="dictionary-import-file-input"]').setInputFiles({ + name: 'valid-dictionary1.zip', + mimeType: 'application/x-zip', + buffer: Buffer.from(dictionary), + }); + await expect(page.locator('id=dictionaries')).toHaveText('Dictionaries (1 installed, 1 enabled)', {timeout: 1 * 60 * 1000}); + + console.log('Open popup-tests.html'); + await page.goto(pathToFileURL(popupTestsPath).toString()); + await page.setViewportSize({width: 1000, height: 4500}); + await expect(page.locator('id=footer')).toBeVisible(); + await page.keyboard.down('Shift'); }); - await expect(page.locator('id=dictionaries')).toHaveText('Dictionaries (1 installed, 1 enabled)', {timeout: 1 * 60 * 1000}); - - /** - * @param {number} doc_number - * @param {number} test_number - * @param {import('@playwright/test').Locator} hovertarget_locator - * @param {{x: number, y: number}} offset - */ - const screenshot = async (doc_number, test_number, hovertarget_locator, offset) => { - const test_name = 'doc' + doc_number + '-test' + test_number; - console.log(test_name); - - const box = (await hovertarget_locator.boundingBox()) || {x: 0, y: 0, width: 0, height: 0}; - - // Find the popup frame if it exists - let popup_frame = page.frames().find((f) => f.url().includes('popup.html')); - - // Otherwise prepare for it to be attached - let frame_attached; - if (typeof popup_frame === 'undefined') { - frame_attached = page.waitForEvent('frameattached'); - } - await page.mouse.move(box.x + offset.x, box.y + offset.y, {steps: 10}); // Hover over the test - if (typeof popup_frame === 'undefined') { - popup_frame = await frame_attached; // Wait for popup to be attached - } - - try { - const expectedState = (await hovertarget_locator.locator('..').getAttribute('data-expected-result')) === 'failure' ? 'hidden' : 'visible'; - await (await /** @type {import('@playwright/test').Frame} */ (popup_frame).frameElement()).waitForElementState(expectedState, {timeout: 500}); - } catch (error) { - console.warn(test_name, 'unexpected popup state'); - } - - console.log(test_name, 'taking screenshot'); - await page.bringToFront(); // Bring the page to the foreground so the screenshot doesn't hang; for some reason the frames result in page being in the background - await expect.soft(page).toHaveScreenshot(test_name + '.png'); - - console.log(test_name, 'clicking away and waiting'); - await page.mouse.click(0, 0); // Click away so popup disappears - await (await /** @type {import('@playwright/test').Frame} */ (popup_frame).frameElement()).waitForElementState('hidden'); // Wait for popup to disappear - }; - - console.log('Open popup-tests.html'); - await page.goto(pathToFileURL(path.join(root, 'test/data/html/popup-tests.html')).toString()); - await page.setViewportSize({width: 1000, height: 4500}); - await expect(page.locator('id=footer')).toBeVisible(); - await page.keyboard.down('Shift'); - let i = 1; - for (const test_locator of await page.locator('.hovertarget').all()) { - await screenshot(2, i, test_locator, {x: 15, y: 15}); - i++; + const popupTestsPath = path.join(root, 'test/data/html/popup-tests.html'); + const numberOfTests = (readFileSync(popupTestsPath, 'utf8').match(/hovertarget/g) || []).length; + for (let i = 1; i <= numberOfTests; i++) { + test(`test${i}`, async ({page}) => { + const test_name = 'doc2-test' + i; + console.log(test_name); + + // Find the test element + const test_locator = page.locator('.hovertarget').nth(i - 1); + + const box = (await test_locator.boundingBox()) || {x: 0, y: 0, width: 0, height: 0}; + + const expectedState = (await test_locator.locator('..').getAttribute('data-expected-result')) === 'failure' ? 'hidden' : 'visible'; + + try { + const frame_attached = page.waitForEvent('frameattached', {timeout: 5000}); + await page.mouse.move(box.x - 5, box.y - 5); // Hover near the test + await page.mouse.move(box.x + 15, box.y + 15, {steps: 10}); // Hover over the test + if (expectedState === 'visible') { + const popup_frame = await frame_attached; + await (await /** @type {import('@playwright/test').Frame} */ (popup_frame).frameElement()) + .waitForElementState(expectedState, {timeout: 500}); + } else { + expect( + await Promise.race([ + frame_attached, + new Promise((resolve) => { + setTimeout(() => resolve('timeout'), 2000); + }), + ]), + ).toStrictEqual('timeout'); + } + } catch (error) { + console.warn(test_name, 'unexpected popup state', error); + } + + console.log(test_name, 'taking screenshot'); + await page.bringToFront(); // Bring the page to the foreground so the screenshot doesn't hang; for some reason the frames result in page being in the background + await expect.soft(page).toHaveScreenshot(test_name + '.png'); + }); } });