Browser SDK Integration Tests #3989

onurtemizkan · 2021-09-19T18:28:57Z

This PR includes the initial structure of new integration tests for Sentry's Browser SDK.

New integration tests internally use Playwright and run on recent versions of Chromium, Webkit and Firefox linked with the Playwright release.

This new test structure aims to create a modern and intuitive environment to test @sentry/browser and potentially other browser-side SDKs like @sentry/react and @sentry/vue.

github-actions · 2021-09-19T18:44:38Z

size-limit report

Path	Base Size (626102f)	Current Size	Change
@sentry/browser - CDN Bundle (gzipped)	22.45 KB	22.44 KB	-0.02% 🔽
@sentry/browser - Webpack	23.29 KB	23.29 KB	+0.01% 🔺
@sentry/browser - Webpack - gzip = false	82.73 KB	82.62 KB	-0.14% 🔽
@sentry/react - Webpack	23.32 KB	23.32 KB	0%
@sentry/nextjs Client - Webpack	48.07 KB	48.03 KB	-0.09% 🔽
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)	29.98 KB	29.96 KB	-0.07% 🔽

AbhiPrasad

nice, yeah I think the test setup is good.

packages/browser/test/e2e/server.js

packages/browser/test/e2e/test/breadcrumbs.test.js

AbhiPrasad · 2021-11-16T14:04:16Z

packages/browser/test/e2e/test/breadcrumbs.test.js

+    expect(eventData.message).toBe('test');
+    expect(eventData.breadcrumbs.length).toBe(1);
+    expect(eventData.breadcrumbs[0].category).toBe('xhr');
+    expect(eventData.breadcrumbs[0].type).toBe('http');


I wonder if it'll be effective to us to pseudo snapshot test the event instead of these deep assertions.

So we can skip attributes that we know will change, but compare the rest of the event to a json file.

We do something similar in electron:

An event JSON: https://github.com/getsentry/sentry-electron/blob/master/test/e2e/test-apps/javascript/main-error/event.json

and how it's normalized: https://github.com/getsentry/sentry-electron/blob/master/test/e2e/recipe/normalize.ts#L49

We can also try this out after this gets merged in, no rush

packages/browser/test/e2e/server.js

packages/browser/test/e2e/playwright.config.js

packages/browser/test/e2e/test/builtins.test.js

packages/browser/test/e2e/test/onunhandledrejection.test.js

.github/workflows/build.yml

packages/browser/package.json

packages/browser/test/e2e/test/config.test.js

packages/browser/test/e2e/test/utils/helpers.js

packages/browser/test/e2e/package.json

.github/workflows/build.yml

packages/browser/test/e2e/scenarios/initializations/default.js

onurtemizkan · 2021-11-30T13:21:58Z

@AbhiPrasad, added a manual about the basic usage and common issues. Will need to be extended further after merging the rest of the tests. The PR is ready to be reviewed again.

rhcarvalho

Thanks @onurtemizkan! I did a pass at this, trying to help us flush this out.

Most comments are small things we can improve or honest questions to help me understand better and also document decisions for our future selves working with and maintaining the code.

My biggest concern for now is around the ability to run individual tests in a single browser in a local development environment.

And then, next, we can strategize how the migration from old tests to new is going to take place as I can see we all need to be careful to not introduce unintended changes (I've seen some unexpected differences and commented).

packages/browser/test/e2e/suites/breadcrumbs/xhr_without_handler/test.ts

packages/browser/test/e2e/README.md

packages/browser/test/e2e/package.json

rhcarvalho · 2021-12-01T13:46:30Z

packages/browser/test/e2e/package.json

+  "main": "index.js",
+  "license": "MIT",
+  "engines": {
+    "node": ">=12"


Is there any advantage in saying >=12 instead of >=16 here? We're configuring the GHA workflow to run on Node 16.x, I don't think we need to declare compatibility with anything else. Thoughts?

packages/browser/test/e2e/utils/helpers.ts

onurtemizkan · 2021-12-03T12:01:35Z

@rhcarvalho, the summary of the latest updates is below;

subject.js is not required / nor the pivot point anymore. We're using test.ts instead, for discovery and building tests.
The improved test is now sentryTest.
page builds are now made before each test, as another extension of sentryTest, (minimal effect on performance now, we'll see better after porting all remaining tests.)
Now we can run individual tests locally on individual browsers. Have access to options in Playwright CLI.
Node >=16 is required now, did not integrate volta though, maybe when we make it a lerna package?
Could not rename the e2e folder to integration now, it depends on when we delete the old tests.

AbhiPrasad

One last set of things, and then I think we are good to go!

packages/eslint-config-sdk/src/index.js

AbhiPrasad · 2021-12-10T17:32:26Z

packages/integration-tests/package.json

@@ -0,0 +1,31 @@
+{
+  "name": "sentry-browser-integration-tests",


Can we call the folder name browser-integration-tests then?

packages/integration-tests/package.json

AbhiPrasad · 2021-12-10T17:33:17Z

packages/integration-tests/package.json

+    "html-webpack-plugin": "^5.5.0",
+    "playwright": "^1.17.1",
+    "ts-loader": "^9.2.6",
+    "typescript": "^4.5.2",


We need to stick with 3.7.5 to match with the rest of the codebase. Can we remove this?

Suggested change

"typescript": "^4.5.2",

@AbhiPrasad, @playwright/test uses export type syntax for its types (and we're importing them). It's not a problem while running tests, but addressing #3989 (comment), to run a type check without compiling before tests, we need a newer version of TS. We also no longer need ts-loader as init and subject scripts are not TS. So the only use of TS here will be static type checking. So is that fine to keep this?

For ref: c3d35b6

Talked with @AbhiPrasad, and we agreed we can address this later in a follow up.

packages/integration-tests/utils/fixtures.ts

packages/integration-tests/utils/generatePage.ts

packages/integration-tests/utils/browserHelpers.ts

Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>

.github/workflows/build.yml

packages/integration-tests/utils/browserHelpers.ts

rhcarvalho · 2021-12-13T08:55:25Z

packages/integration-tests/webpack.config.ts

+            // This has a big impact on test build speed.
+            transpileOnly: true,


I see this option documented in https://webpack.js.org/guides/build-performance/#typescript-loader

I could not find where we're actually doing the type-checking?

Added a non-compiled type checking script and removed ts-loader from webpack which we don't need anymore. -> c3d35b6

But we need a higher version of TS for this. -> #3989 (comment)

rhcarvalho · 2021-12-13T09:56:23Z

For our later reference, the last Playwright tests took 1m6s, of which 59s was downloading browsers/system dependencies (and as per #3989 (comment) I think we can get rid of this step and save time).

packages/integration-tests/package.json

package.json

Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>

…tsentry/sentry-javascript into onur/playwright-browser-tests

… into onur/playwright-browser-tests

rhcarvalho · 2021-12-14T13:48:45Z

Ok, we got now the test failure in 3 browsers as expected:

https://github.com/getsentry/sentry-javascript/runs/4520662297?check_suite_focus=true

However, tests are not running in Webkit due to missing system level dependencies -- we need to apply a fix similar to https://github.com/getsentry/sentry-javascript/pull/4232/files#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721R304

All feedback has been addressed or will be addressed in a follow up

rhcarvalho · 2021-12-14T14:11:08Z

Now we have all browsers testing and failing as expected https://github.com/getsentry/sentry-javascript/runs/4521042164?check_suite_focus=true.

Tests flipped to green in 84f529b and we should get it merged as soon as the new run turn green.

rhcarvalho

Thanks @onurtemizkan! Looking forward to what we build on top of this ❤️

onurtemizkan · 2021-12-14T14:27:24Z

Thanks everyone for the reviews! ❤️ 🚀

Initial structure of new integration tests for Sentry's Browser SDK. New integration tests internally use Playwright and run on recent versions of Chromium, Webkit and Firefox linked with the Playwright release. This new test structure aims to create a modern and intuitive environment to test @sentry/browser and potentially other browser-side SDKs like @sentry/react and @sentry/vue. Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io> Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>

onurtemizkan changed the title ~~[WIP] - test(browser): Simple PoC for end-to-end browser tests using Playwright.~~ [WIP] - Playwright E2E Tests. Sep 19, 2021

onurtemizkan force-pushed the onur/playwright-browser-tests branch 4 times, most recently from 3abadea to 50e1b56 Compare September 20, 2021 10:59

onurtemizkan marked this pull request as ready for review September 22, 2021 13:08

onurtemizkan requested a review from kamilogorek as a code owner September 22, 2021 13:08

onurtemizkan force-pushed the onur/playwright-browser-tests branch 3 times, most recently from 3aaaf76 to c28ceac Compare November 9, 2021 08:57

onurtemizkan mentioned this pull request Nov 12, 2021

fix(browser): Set : as a part of gecko protocol regex group. #4153

Merged

AbhiPrasad requested review from AbhiPrasad and removed request for kamilogorek November 12, 2021 15:49

AbhiPrasad reviewed Nov 16, 2021

View reviewed changes

vladanpaunovic reviewed Nov 16, 2021

View reviewed changes

iker-barriocanal reviewed Nov 17, 2021

View reviewed changes

onurtemizkan force-pushed the onur/playwright-browser-tests branch 3 times, most recently from 195f6e2 to 0a47492 Compare November 23, 2021 12:00

AbhiPrasad reviewed Nov 23, 2021

View reviewed changes

packages/browser/test/e2e/scenarios/initializations/default.js Outdated Show resolved Hide resolved

onurtemizkan force-pushed the onur/playwright-browser-tests branch 3 times, most recently from 1dcd4e0 to 0e2a049 Compare November 24, 2021 22:59

onurtemizkan changed the title ~~[WIP] - Playwright E2E Tests.~~ Browser SDK Integration Tests Nov 30, 2021

rhcarvalho reviewed Dec 1, 2021

View reviewed changes

smeubank added this to the Release Stability milestone Dec 1, 2021

smeubank mentioned this pull request Dec 1, 2021

Extract/generalize Playwright int framework #4208

Closed

rhcarvalho added 2 commits December 10, 2021 16:17

Rename job for consistency

05fa814

Markdown formatting

9dc6920

AbhiPrasad previously requested changes Dec 10, 2021

View reviewed changes

onurtemizkan and others added 5 commits December 13, 2021 07:57

Update packages/integration-tests/package.json

bc40012

Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>

Merge branch 'master' into onur/playwright-browser-tests

a262fd6

Update packages/integration-tests/utils/fixtures.ts

0633085

Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>

Exclude dist from eslint checks.

26352af

Address review suggestions.

26ca39f

rhcarvalho reviewed Dec 13, 2021

View reviewed changes

packages/integration-tests/package.json Outdated Show resolved Hide resolved

package.json Outdated Show resolved Hide resolved

onurtemizkan and others added 7 commits December 14, 2021 09:28

Remove repeated browser installation

e668345

Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>

Remove browserHelpers

022faff

Merge branch 'onur/playwright-browser-tests' of https://github.com/ge…

4b25eb4

…tsentry/sentry-javascript into onur/playwright-browser-tests

Rename project as @sentry-internal/browser-integration-tests.

06c95fb

Remove obsolete ts-loader, enable TS type checks before tests.

c3d35b6

Merge branch 'master' of https://github.com/getsentry/sentry-javascript…

84138d2

… into onur/playwright-browser-tests

Use test:ci on GitHub actions.

5d7601a

Install webkit deps on run.

9faaa47

rhcarvalho enabled auto-merge (squash) December 14, 2021 14:07

onurtemizkan added 2 commits December 14, 2021 14:08

Flip sample test to pass.

84f529b

Merge branch 'master' into onur/playwright-browser-tests

089a8eb

rhcarvalho approved these changes Dec 14, 2021

View reviewed changes

rhcarvalho merged commit 28dae68 into master Dec 14, 2021

rhcarvalho deleted the onur/playwright-browser-tests branch December 14, 2021 14:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Browser SDK Integration Tests #3989

Browser SDK Integration Tests #3989

onurtemizkan commented Sep 19, 2021 •

edited

Loading

github-actions bot commented Sep 19, 2021 •

edited

Loading

AbhiPrasad left a comment

AbhiPrasad Nov 16, 2021

onurtemizkan commented Nov 30, 2021

rhcarvalho left a comment

rhcarvalho Dec 1, 2021

onurtemizkan commented Dec 3, 2021

AbhiPrasad left a comment

AbhiPrasad Dec 10, 2021

AbhiPrasad Dec 10, 2021

onurtemizkan Dec 14, 2021

onurtemizkan Dec 14, 2021

rhcarvalho Dec 14, 2021

rhcarvalho Dec 13, 2021

onurtemizkan Dec 14, 2021 •

edited

Loading

rhcarvalho commented Dec 13, 2021

rhcarvalho commented Dec 14, 2021

rhcarvalho commented Dec 14, 2021

rhcarvalho left a comment

onurtemizkan commented Dec 14, 2021

		@@ -0,0 +1,31 @@
		{
		"name": "sentry-browser-integration-tests",

		// This has a big impact on test build speed.
		transpileOnly: true,

Browser SDK Integration Tests #3989

Browser SDK Integration Tests #3989

Conversation

onurtemizkan commented Sep 19, 2021 • edited Loading

github-actions bot commented Sep 19, 2021 • edited Loading

size-limit report

AbhiPrasad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onurtemizkan commented Nov 30, 2021

rhcarvalho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onurtemizkan commented Dec 3, 2021

AbhiPrasad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onurtemizkan Dec 14, 2021 • edited Loading

Choose a reason for hiding this comment

rhcarvalho commented Dec 13, 2021

rhcarvalho commented Dec 14, 2021

rhcarvalho commented Dec 14, 2021

rhcarvalho left a comment

Choose a reason for hiding this comment

onurtemizkan commented Dec 14, 2021

onurtemizkan commented Sep 19, 2021 •

edited

Loading

github-actions bot commented Sep 19, 2021 •

edited

Loading

onurtemizkan Dec 14, 2021 •

edited

Loading