ci(e2e): e2e with playwright #1171

subzero10 · 2023-08-25T11:25:25Z

Status

Ready

Description

I decided to give playwright a go and see how it performs versus our current integration tests.

Run tests on Browserstack from Playwright. Running on minimum and latest versions. Supported browsers: chrome and edge.
Run tests on local browser executables (shipped with playwright): Firefox, Chrome, Edge, Safari, Chromium and mobile browsers

Karma vs Playwright

Karma/Jasmine

Pros:

runs on Browserstack, supports all browsers and browser versions available on Browserstack

Cons:

feels outdated
tests are becoming more and more flaky
lots of usage of setTimeout (especially since the switch to promises from the callback-based approach)
difficult to debug

Playwright

Pros:

modern api
fast (even faster when not connected to remote browsers on browserstack)
support for flaky tests
local browser testing (installed through npm)
browserstack support (though limited to chrome and edge)

Cons:

recommended approach is to test only on latest browser versions, but we also want to test on older versions

Todos

Migrate all tests to playwright
Browserstack integration
Karma/Jasmine vs Playwright comparison
README.md - explain the minimal architecture (server.js)

# Conflicts: # packages/js/package-lock.json

subzero10 · 2023-09-08T10:26:32Z

Tests are passing, but we now have "2 flaky tests" tagged from playwright.
I will look at them separately.

BethanyBerkowitz

This is exciting! I'm not done testing it out, but I wanted to post what I had so far.

I did run into a problem in testing, which is that if you ctrl+C out of running the tests, it leaves bsLocal running, for me it was on port 45690. Then if you try running them again, Browserstack can't start because there's already something running on the port.

packages/js/README.md

packages/js/test/e2e/browsers.ts

packages/js/test/e2e/browserstack.config.ts

packages/js/test/e2e/browsers.ts

packages/js/test/e2e/browserstack.config.ts

packages/js/test/e2e/global-setup.ts

subzero10 · 2023-09-11T17:58:01Z

I did run into a problem in testing, which is that if you ctrl+C out of running the tests, it leaves bsLocal running, for me it was on port 45690. Then if you try running them again, Browserstack can't start because there's already something running on the port.

Fixed!

# Conflicts: # packages/js/package-lock.json

subzero10 · 2023-09-11T18:28:15Z

@BethanyBerkowitz Thanks for the detailed review. I went through all your comments. Feel free to go for a new review round :)

subzero10 · 2023-09-11T18:30:52Z

Also, I added this #1188 to look at the flaky tests after we merge this.

joshuap

Love it! Especially the pros/cons writeup—thanks for that. I'm sold on playwright. :)

tbh, I'd be OK with testing latest browsers if we ever had to compromise, but it's cool that we can also do the minimum versions. 👌

subzero10 · 2023-09-12T06:30:04Z

tbh, I'd be OK with testing latest browsers if we ever had to compromise, but it's cool that we can also do the minimum versions. 👌

💯

subzero10 · 2023-09-12T06:31:02Z

I'll wait for @BethanyBerkowitz's review before merging, since there were more than a few changes since the first review 😄

BethanyBerkowitz

Yay, thanks so much @subzero10!

I still get test failures when running locally, but since you've got a separate issue open for flakiness I think this one is ready to go.

subzero10 · 2023-09-14T22:33:46Z

I still get test failures when running locally, but since you've got a separate issue open for flakiness I think this one is ready to go.

I'm curious, have you tried running the tests only with playwright (i.e. disable the browserstack browsers and unset any browserstack env variables)?

BethanyBerkowitz · 2023-09-15T16:37:32Z

I still get test failures when running locally, but since you've got a separate issue open for flakiness I think this one is ready to go.

I'm curious, have you tried running the tests only with playwright (i.e. disable the browserstack browsers and unset any browserstack env variables)?

Mmm yeah I still get a bunch of failures -- 36 on my most recent run 🤔

36 failed
    [chromium] › integration.spec.ts:392:7 › Browser Integration › it sends user feedback for notice on submit 
    [chromium] › integration.spec.ts:438:7 › Browser Integration › it closes user feedback form on cancel 
    [firefox] › integration.spec.ts:330:7 › Browser Integration › it shows user feedback form 
    [firefox] › integration.spec.ts:359:7 › Browser Integration › it shows user feedback form with custom labels 
    [firefox] › integration.spec.ts:392:7 › Browser Integration › it sends user feedback for notice on submit 
    [firefox] › integration.spec.ts:438:7 › Browser Integration › it closes user feedback form on cancel 
    [webkit] › integration.spec.ts:392:7 › Browser Integration › it sends user feedback for notice on submit 
    [webkit] › integration.spec.ts:438:7 › Browser Integration › it closes user feedback form on cancel 
    [Mobile Chrome] › integration.spec.ts:392:7 › Browser Integration › it sends user feedback for notice on submit 
    [Mobile Chrome] › integration.spec.ts:438:7 › Browser Integration › it closes user feedback form on cancel 
    [Mobile Safari] › integration.spec.ts:392:7 › Browser Integration › it sends user feedback for notice on submit 
    [Microsoft Edge] › integration.spec.ts:52:7 › Browser Integration › it logs browser type and version 
    [Microsoft Edge] › integration.spec.ts:58:7 › Browser Integration › it notifies Honeybadger of unhandled exceptions 
    [Microsoft Edge] › integration.spec.ts:66:7 › Browser Integration › it notifies Honeybadger manually 
    [Microsoft Edge] › integration.spec.ts:76:7 › Browser Integration › it reports multiple errors in the same process 
    [Microsoft Edge] › integration.spec.ts:91:7 › Browser Integration › it sends console breadcrumbs 
    [Microsoft Edge] › integration.spec.ts:105:7 › Browser Integration › it sends string value console breadcrumbs when null 
    [Microsoft Edge] › integration.spec.ts:119:7 › Browser Integration › it sends click breadcrumbs 
    [Microsoft Edge] › integration.spec.ts:136:7 › Browser Integration › it sends XHR breadcrumbs for relative paths 
    [Microsoft Edge] › integration.spec.ts:161:7 › Browser Integration › it sends XHR breadcrumbs for absolute paths 
    [Microsoft Edge] › integration.spec.ts:186:7 › Browser Integration › it sends fetch breadcrumbs 
    [Microsoft Edge] › integration.spec.ts:204:7 › Browser Integration › it sends navigation breadcrumbs 
    [Microsoft Edge] › integration.spec.ts:222:7 › Browser Integration › it sends notify breadcrumbs 
    [Microsoft Edge] › integration.spec.ts:241:7 › Browser Integration › it sends window.onerror breadcrumbs 
    [Microsoft Edge] › integration.spec.ts:260:7 › Browser Integration › it skips onunhandledrejection when already sent 
    [Microsoft Edge] › integration.spec.ts:277:7 › Browser Integration › it sends window.onunhandledrejection breadcrumbs when rejection is an Error 
    [Microsoft Edge] › integration.spec.ts:308:7 › Browser Integration › it skips window.onunhandledrejection breadcrumbs when rejection is not Error 
    [Microsoft Edge] › integration.spec.ts:330:7 › Browser Integration › it shows user feedback form 
    [Microsoft Edge] › integration.spec.ts:359:7 › Browser Integration › it shows user feedback form with custom labels 
    [Microsoft Edge] › integration.spec.ts:392:7 › Browser Integration › it sends user feedback for notice on submit 
    [Microsoft Edge] › integration.spec.ts:438:7 › Browser Integration › it closes user feedback form on cancel 
    [Microsoft Edge] › integration.spec.ts:470:7 › Web Worker Integration › it creates worker 
    [Google Chrome] › integration.spec.ts:330:7 › Browser Integration › it shows user feedback form 
    [Google Chrome] › integration.spec.ts:359:7 › Browser Integration › it shows user feedback form with custom labels 
    [Google Chrome] › integration.spec.ts:392:7 › Browser Integration › it sends user feedback for notice on submit 
    [Google Chrome] › integration.spec.ts:438:7 › Browser Integration › it closes user feedback form on cancel 
  111 passed (3.9m)

subzero10 · 2023-09-17T18:30:45Z

That's very strange. Which nodejs version are you using?

BethanyBerkowitz · 2023-09-18T14:47:24Z

That's very strange. Which nodejs version are you using?

Looks like I was on 16.13.0, but I also get a bunch of failures on 20.3.1.

BethanyBerkowitz · 2023-09-18T15:11:00Z

@subzero10 I tried running the playwright tests just in Firefox using --ui so I could get a better sense of what's going on. It looks like for some reason the user feedback form is added to the DOM twice.

Happy to look further into this, but I figured you have much more knowledge than me in this area and may have a sense of what's wrong?

subzero10 · 2023-09-19T05:30:39Z

That's very strange. Which nodejs version are you using?

Looks like I was on 16.13.0, but I also get a bunch of failures on 20.3.1.

FYI, I'm running on node 16 as well.

subzero10 · 2023-09-19T05:31:46Z

@subzero10 I tried running the playwright tests just in Firefox using --ui so I could get a better sense of what's going on. It looks like for some reason the user feedback form is added to the DOM twice.

Happy to look further into this, but I figured you have much more knowledge than me in this area and may have a sense of what's wrong?

Thanks for the insight! Yes, I can look into it! I remember adding a safeguard about this but it may not be working all the time.

subzero10 · 2023-09-19T06:10:40Z

OK, I know why this could happen, but I don't understand why.
For the tests, I had to disable the safeguard (isUserFeedbackScriptUrlAlreadyVisible) because I inject the feedback form script in a different way:

const handle = await page.evaluateHandle(_ => {
      // DISABLE SAFEGUARD AND SCRIPT LOADING FUNCTIONS
      window.Honeybadger.isUserFeedbackScriptUrlAlreadyVisible = () => false
      window.Honeybadger.appendUserFeedbackScriptTag = () => { }
      // ... ...
})
// ... ...

// LOAD SCRIPT USING PLAYWRIGHT
const relativePath = '../../dist/browser/honeybadger-feedback-form.js'
await page.addScriptTag({
      path: resolve(__dirname, relativePath)
})
// ... ...

However, I don't see why the addScriptTag would be called twice, or result in the script being added twice.
The only theory that could come to mind is that the page for the current test is the same as the page from the previous test, that already had the feedback form script loaded. But that can't be true, because each test is isolated with a fresh page. By the way, I can't reproduce this behavior 🤷‍♂️.

BethanyBerkowitz · 2023-09-21T15:45:50Z

OK, I know why this could happen, but I don't understand why. For the tests, I had to disable the safeguard (isUserFeedbackScriptUrlAlreadyVisible) because I inject the feedback form script in a different way:
const handle = await page.evaluateHandle(_ => {
      // DISABLE SAFEGUARD AND SCRIPT LOADING FUNCTIONS
      window.Honeybadger.isUserFeedbackScriptUrlAlreadyVisible = () => false
      window.Honeybadger.appendUserFeedbackScriptTag = () => { }
      // ... ...
})
// ... ...

// LOAD SCRIPT USING PLAYWRIGHT
const relativePath = '../../dist/browser/honeybadger-feedback-form.js'
await page.addScriptTag({
      path: resolve(__dirname, relativePath)
})
// ... ...
However, I don't see why the addScriptTag would be called twice, or result in the script being added twice. The only theory that could come to mind is that the page for the current test is the same as the page from the previous test, that already had the feedback form script loaded. But that can't be true, because each test is isolated with a fresh page. By the way, I can't reproduce this behavior 🤷‍♂️.

Well, I'm not sure what you did in #1190 that could fix my problem, but the Firefox tests no longer error for me!

ci(e2e): e2e with playwright

6ed502c

subzero10 self-assigned this Aug 25, 2023

subzero10 force-pushed the e2e-with-playwright branch from 8efa659 to 4ad6263 Compare August 25, 2023 11:39

chore: fix ci

c14661a

subzero10 force-pushed the e2e-with-playwright branch from 4ad6263 to c14661a Compare August 25, 2023 14:29

chore: migrate all karma tests to playwright

717755a

subzero10 force-pushed the e2e-with-playwright branch from 3808b66 to 717755a Compare August 26, 2023 14:18

subzero10 added 14 commits August 26, 2023 17:23

chore: remove before all

55edddc

chore: fix integration test

817c487

feat: playwright and browserstack

bce391d

chore: specify chrome and edge on browserstack browsers

25188b8

chore: browserstack secrets on ci

6fc4a80

Merge branch 'master' into e2e-with-playwright

5a11c7a

# Conflicts: # packages/js/package-lock.json

chore: debug browserstack not working

389ea52

chore: enable network logs

c01c4fd

chore: set browserstack local to true

b8aaeae

chore: trigger e2e

51238ea

chore: run on latest chrome

df0c0b8

chore: dummy commit

a09533d

chore: run playwright browsers

16e7a39

chore: run both browserstack and playwright

3275fc9

subzero10 requested review from joshuap and BethanyBerkowitz September 8, 2023 10:19

chore: typo

5350352

BethanyBerkowitz reviewed Sep 8, 2023

View reviewed changes

chore: stop bsLocal on SIGINT

52f122f

subzero10 added 2 commits September 11, 2023 21:12

chore: apply PR review feedback

0711866

chore: refactor browserstack config

aaab352

Merge branch 'master' into e2e-with-playwright

1077557

# Conflicts: # packages/js/package-lock.json

subzero10 requested a review from BethanyBerkowitz September 11, 2023 18:26

chore: fix ts

17fea36

subzero10 mentioned this pull request Sep 11, 2023

Flaky integration tests reported with Playwright #1188

Open

joshuap approved these changes Sep 11, 2023

View reviewed changes

BethanyBerkowitz approved these changes Sep 14, 2023

View reviewed changes

subzero10 merged commit fa6a6d4 into master Sep 14, 2023

subzero10 deleted the e2e-with-playwright branch September 14, 2023 22:32

subzero10 mentioned this pull request Sep 15, 2023

ci(e2e): fix setup #1190

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci(e2e): e2e with playwright #1171

ci(e2e): e2e with playwright #1171

subzero10 commented Aug 25, 2023 •

edited

Loading

subzero10 commented Sep 8, 2023

BethanyBerkowitz left a comment

subzero10 commented Sep 11, 2023

subzero10 commented Sep 11, 2023

subzero10 commented Sep 11, 2023 •

edited

Loading

joshuap left a comment

subzero10 commented Sep 12, 2023

subzero10 commented Sep 12, 2023

BethanyBerkowitz left a comment

subzero10 commented Sep 14, 2023

BethanyBerkowitz commented Sep 15, 2023

subzero10 commented Sep 17, 2023

BethanyBerkowitz commented Sep 18, 2023

BethanyBerkowitz commented Sep 18, 2023

subzero10 commented Sep 19, 2023

subzero10 commented Sep 19, 2023

subzero10 commented Sep 19, 2023

BethanyBerkowitz commented Sep 21, 2023

ci(e2e): e2e with playwright #1171

ci(e2e): e2e with playwright #1171

Conversation

subzero10 commented Aug 25, 2023 • edited Loading

Status

Description

Karma vs Playwright

Todos

subzero10 commented Sep 8, 2023

BethanyBerkowitz left a comment

Choose a reason for hiding this comment

subzero10 commented Sep 11, 2023

subzero10 commented Sep 11, 2023

subzero10 commented Sep 11, 2023 • edited Loading

joshuap left a comment

Choose a reason for hiding this comment

subzero10 commented Sep 12, 2023

subzero10 commented Sep 12, 2023

BethanyBerkowitz left a comment

Choose a reason for hiding this comment

subzero10 commented Sep 14, 2023

BethanyBerkowitz commented Sep 15, 2023

subzero10 commented Sep 17, 2023

BethanyBerkowitz commented Sep 18, 2023

BethanyBerkowitz commented Sep 18, 2023

subzero10 commented Sep 19, 2023

subzero10 commented Sep 19, 2023

subzero10 commented Sep 19, 2023

BethanyBerkowitz commented Sep 21, 2023

subzero10 commented Aug 25, 2023 •

edited

Loading

subzero10 commented Sep 11, 2023 •

edited

Loading