🧪 Update browser matrix for tests #2884

thomas-lebeau · 2024-07-22T14:20:32Z

Motivation

Until now we're testing only on somewhat recent version off different browsers. However, as we're defining a new browser support matrix I thought it would be better to test on the actual minimum version of the browsers we're supporting.

This PR only focus on unit tests. I'll investigate separately for e2e tests as it's a bit more complicated.

Changes

separate unit and e2e browser matrix
use new browser matrix for unit tests.
properly check feature availability before running unit tests
refactor onunhandledrejection to handle the case where it's not supported (Firefox 67 & 68)

Testing

Local
Staging
Unit
End to end

I have gone over the contributing documentation.

cit-pr-commenter · 2024-07-22T14:29:47Z

Bundles Sizes Evolution

📦 Bundle Name	Base Size	Local Size	𝚫	𝚫%	Status
Rum	158.86 KiB	158.92 KiB	57 B	+0.04%	✅
Logs	55.74 KiB	55.75 KiB	11 B	+0.02%	✅
Rum Slim	107.47 KiB	107.53 KiB	57 B	+0.05%	✅
Worker	25.21 KiB	25.21 KiB	0 B	0.00%	✅

🚀 CPU Performance

Action Name	Base Average Cpu Time (ms)	Local Average Cpu Time (ms)	𝚫
addglobalcontext	0.002	0.001	-0.001
addaction	0.049	0.035	-0.014
adderror	0.053	0.037	-0.016
addtiming	0.001	0.001	-0.000
startview	1.272	1.001	-0.271
startstopsessionreplayrecording	1.485	0.798	-0.688
logmessage	0.030	0.019	-0.010

🧠 Memory Performance

Action Name	Base Consumption Memory (bytes)	Local Consumption Memory (bytes)	𝚫 (bytes)
addglobalcontext	21.42 KiB	20.59 KiB	-848 B
addaction	69.60 KiB	71.88 KiB	2.27 KiB
adderror	84.54 KiB	87.78 KiB	3.24 KiB
addtiming	17.93 KiB	19.67 KiB	1.75 KiB
startview	347.79 KiB	351.50 KiB	3.71 KiB
startstopsessionreplayrecording	13.47 KiB	13.02 KiB	-464 B
logmessage	69.32 KiB	72.06 KiB	2.73 KiB

🔗 RealWorld

codecov-commenter · 2024-07-24T07:28:27Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.67%. Comparing base (6dc4216) to head (569a30e).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2884      +/-   ##
==========================================
+ Coverage   93.65%   93.67%   +0.01%     
==========================================
  Files         268      268              
  Lines        7584     7584              
  Branches     1686     1687       +1     
==========================================
+ Hits         7103     7104       +1     
+ Misses        481      480       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

thomas-lebeau · 2024-07-25T08:37:37Z

packages/core/test/emulate/mockStorages.ts

+    getSpy: spyOnProperty(document, 'cookie', 'get').and.callFake(() => cookie),
+    setSpy: spyOnProperty(document, 'cookie', 'set').and.callFake((newCookie) => (cookie = newCookie)),


fix for Firefox 67

thomas-lebeau · 2024-07-25T08:40:05Z

packages/core/src/tools/serialisation/sanitize.spec.ts

@@ -46,7 +46,7 @@ describe('sanitize', () => {
      const symbolFunction: (description: string) => any = (window as any).Symbol
      if (typeof symbolFunction === 'function') {
        const symbol = symbolFunction('description')
-        expect(sanitize(symbol)).toEqual('[Symbol] description')
+        expect(sanitize(symbol)).toMatch(/\[Symbol\] (?:Symbol\()?description\)?/)


this match better the implementation:

return `[Symbol] ${(value as symbolWithDescription).description || value.toString()}

which will return [Symbol] foo if Symbol.description is supported and [Symbol] Symbol(foo)otherwise

thomas-lebeau · 2024-07-25T08:43:01Z

packages/core/src/tools/instrumentMethod.ts

@@ -77,7 +77,7 @@ export function instrumentMethod<TARGET extends { [key: string]: any }, METHOD e
  let original = targetPrototype[method]

  if (typeof original !== 'function') {
-    if (startsWith(method, 'on')) {
+    if (method in targetPrototype && startsWith(method, 'on')) {


if the event is not supported (e.g 'onunhandledrejection' in window) we should replace it.

thomas-lebeau · 2024-07-25T09:53:44Z

packages/logs/src/domain/networkError/networkErrorCollection.spec.ts

-  let onunhandledrejectionSpy: jasmine.Spy
-
-  beforeEach(() => {
-    onunhandledrejectionSpy = jasmine.createSpy()
-    window.onunhandledrejection = onunhandledrejectionSpy
-
-    registerCleanupTask(() => {
-      window.onunhandledrejection = null
-    })
-  })
-


thomas-lebeau · 2024-07-25T09:53:52Z

packages/rum-core/src/domain/requestCollection.spec.ts

-    window.onunhandledrejection = (ev: PromiseRejectionEvent) => {
-      throw new Error(`unhandled rejected promise \n    ${ev.reason as string}`)
-    }


thomas-lebeau · 2024-07-26T10:32:14Z

/to-staging

dd-devflow · 2024-07-26T10:32:21Z

🚂 Branch Integration: starting soon, median merge time is 11m

Commit 111f94454a will soon be integrated into staging-30.

Use /to-staging -c to cancel this operation!

dd-devflow · 2024-07-26T10:32:28Z

🚨 Branch Integration: This merge request has conflicts

We couldn't automatically merge the commit 111f94454a into staging-30!

You can use this resolution PR: #2898 to fix the conflicts.

If you need support, contact us on Slack #devflow with those details!

thomas-lebeau · 2024-07-26T11:00:55Z

packages/rum-core/src/domain/view/viewMetrics/trackCumulativeLayoutShift.ts

@@ -138,5 +137,5 @@ function slidingSessionWindow() {
 * Check whether `layout-shift` is supported by the browser.
 */
 export function isLayoutShiftSupported() {
-  return supportPerformanceTimingEvent(RumPerformanceEntryType.LAYOUT_SHIFT)
+  return supportPerformanceTimingEvent(RumPerformanceEntryType.LAYOUT_SHIFT) && 'WeakRef' in window


We used WeakRef to avoid memory leak when tracking CLS vital and assumed that browsers supporting layout-shift where also supporting WeakRef. Unfortunately that's not true and there is a few browsers versions that does support layout shift but not WeakRef. Unfortunately we were never testing on such browsers :(

This bug affect the following browsers:

Chrome 77 to 83

Edge 79 to 83

Opera 64 to 69

Not sure how this is communicated, but it's worth mentioning in our documentation I guess

I agree, although currently there is a lot of gaps on how we document the features supported by browsers and does not includes version.

My RFC around dropping IE11 proposes to remediates to this issue:

Additionally we can update the feature support by browser to also include specific version numbers.

packages/core/src/domain/error/trackRuntimeError.spec.ts

N-Boutaib · 2024-07-26T11:38:05Z

packages/logs/src/domain/networkError/networkErrorCollection.spec.ts

👏 Praise: Thanks for the cleanup

N-Boutaib · 2024-07-26T11:41:10Z

packages/rum-core/src/domain/view/viewMetrics/trackCumulativeLayoutShift.ts

@@ -138,5 +137,5 @@ function slidingSessionWindow() {
 * Check whether `layout-shift` is supported by the browser.
 */
 export function isLayoutShiftSupported() {
-  return supportPerformanceTimingEvent(RumPerformanceEntryType.LAYOUT_SHIFT)
+  return supportPerformanceTimingEvent(RumPerformanceEntryType.LAYOUT_SHIFT) && 'WeakRef' in window


Not sure how this is communicated, but it's worth mentioning in our documentation I guess

dd-devflow · 2024-07-26T15:16:44Z

🚂 Branch Integration: starting soon, median merge time is 11m

Commit 111f94454a will soon be integrated into staging-30.

dd-devflow · 2024-07-26T15:26:11Z

🚨 Branch integration failed

Have a look at the resolution PR #2898 for more information.

dd-devflow · 2024-07-26T15:42:26Z

🚂 Branch Integration: starting soon, median merge time is 11m

Commit 111f94454a will soon be integrated into staging-30.

dd-devflow · 2024-07-26T15:51:41Z

🚨 Branch integration failed

Have a look at the resolution PR #2898 for more information.

thomas-lebeau · 2024-07-30T07:43:28Z

/to-staging

dd-devflow · 2024-07-30T07:43:34Z

🚂 Branch Integration: starting soon, median merge time is 0s

Commit efe8b8cd27 will soon be integrated into staging-31.

Use /to-staging -c to cancel this operation!

…ng-31 Integrated commit sha: efe8b8c Co-authored-by: Thomas Lebeau <thomas.lebeau@datadoghq.com>

dd-devflow · 2024-07-30T07:54:56Z

🚂 Branch Integration: This commit was successfully integrated

Commit efe8b8cd27 has been merged into staging-31 in merge commit 367f4caa30.

Check out the triggered pipeline on Gitlab 🦊

BenoitZugmeyer · 2024-07-30T10:20:23Z

test/browsers.unit.conf.js

Suggestion:

move browsers.e2e.conf.js to e2e/browsers.conf.js

move browsers.unit.conf.js to unit/browsers.conf.js

remove unnecessary entries from browsers used by e2e tests (see

browser-sdk/test/e2e/wdio.bs.conf.ts

Lines 12 to 19 in efe8b8c

.filter(

(configuration) =>

configuration.sessionName !== 'IE' &&

// Safari mobile on iOS <= 14.0 does not support

// the way we flush events on page change

// TODO check newer version on browserstack

configuration.sessionName !== 'Safari mobile'

)

)

pm_trace_id: 40455230 feature_branch_pipeline_id: 40455230 source: to-staging * commit 'fa9bb6a410d74e6371b6d5de41b230878c2c2768': [RUM-5590] Add telemetry for INP null target (#2895) 🧪 Update browser matrix for tests (#2884)

thomas-lebeau force-pushed the thomas.lebeau/update-test-browser-matrix branch 7 times, most recently from 13b2419 to 8bb6673 Compare July 25, 2024 09:44

thomas-lebeau commented Jul 25, 2024

View reviewed changes

thomas-lebeau force-pushed the v6 branch from 73da19d to 612c73e Compare July 25, 2024 10:03

thomas-lebeau force-pushed the thomas.lebeau/update-test-browser-matrix branch from 8bb6673 to 0ecbfef Compare July 25, 2024 10:05

thomas-lebeau force-pushed the v6 branch 2 times, most recently from 1ab31a8 to 612c73e Compare July 25, 2024 15:14

thomas-lebeau force-pushed the thomas.lebeau/update-test-browser-matrix branch from 0ecbfef to 7e10985 Compare July 26, 2024 04:53

thomas-lebeau changed the base branch from v6 to main July 26, 2024 09:00

thomas-lebeau force-pushed the thomas.lebeau/update-test-browser-matrix branch from 7e10985 to c051ad0 Compare July 26, 2024 09:08

dd-devflow bot added a commit that referenced this pull request Jul 26, 2024

[skip ci] #2884 conflicts with staging-30

72a787e

dd-devflow bot mentioned this pull request Jul 26, 2024

[fixme] #2884 conflicts with staging-30 #2898

Closed

thomas-lebeau marked this pull request as ready for review July 26, 2024 10:46

thomas-lebeau requested a review from a team as a code owner July 26, 2024 10:46

thomas-lebeau commented Jul 26, 2024

View reviewed changes

N-Boutaib reviewed Jul 26, 2024

View reviewed changes

N-Boutaib approved these changes Jul 26, 2024

View reviewed changes

N-Boutaib requested a review from a team July 26, 2024 12:35

thomas-lebeau force-pushed the thomas.lebeau/update-test-browser-matrix branch from fdc61b0 to fe33e68 Compare July 29, 2024 17:51

thomas-lebeau added 5 commits July 29, 2024 20:05

🧪 Update browser matrix for tests

5966263

✅ fix unit tests for older browsers

54d9757

♻️ refactor onunhandledrejection

5f9fc31

improve spy on onunhandledrejection

bf96e1b

do not override onunhandledrejection if it does not exsits

58b2726

thomas-lebeau force-pushed the thomas.lebeau/update-test-browser-matrix branch from fe33e68 to 58b2726 Compare July 29, 2024 18:05

add back ie11 for unit tests

efe8b8c

dd-mergequeue bot added a commit that referenced this pull request Jul 30, 2024

Integrate thomas.lebeau/update-test-browser-matrix (#2884) into stagi…

367f4ca

…ng-31 Integrated commit sha: efe8b8c Co-authored-by: Thomas Lebeau <thomas.lebeau@datadoghq.com>

dd-devflow bot added the staging-31 label Jul 30, 2024

BenoitZugmeyer approved these changes Jul 30, 2024

View reviewed changes

👌 cleanup from PR review comments

569a30e

thomas-lebeau merged commit 9d19ead into main Jul 30, 2024
20 checks passed

thomas-lebeau deleted the thomas.lebeau/update-test-browser-matrix branch July 30, 2024 11:22

thomas-lebeau added a commit that referenced this pull request Jul 30, 2024

🧪 Update browser matrix for tests (#2884)

7611b50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🧪 Update browser matrix for tests #2884

🧪 Update browser matrix for tests #2884

thomas-lebeau commented Jul 22, 2024 •

edited

Loading

cit-pr-commenter bot commented Jul 22, 2024 •

edited

Loading

codecov-commenter commented Jul 24, 2024 •

edited

Loading

thomas-lebeau Jul 25, 2024

thomas-lebeau Jul 25, 2024

thomas-lebeau Jul 25, 2024

thomas-lebeau Jul 25, 2024

thomas-lebeau Jul 25, 2024

thomas-lebeau commented Jul 26, 2024

dd-devflow bot commented Jul 26, 2024

dd-devflow bot commented Jul 26, 2024

thomas-lebeau Jul 26, 2024

N-Boutaib Jul 26, 2024

thomas-lebeau Jul 26, 2024

N-Boutaib Jul 26, 2024

N-Boutaib Jul 26, 2024

dd-devflow bot commented Jul 26, 2024

dd-devflow bot commented Jul 26, 2024

dd-devflow bot commented Jul 26, 2024

dd-devflow bot commented Jul 26, 2024

thomas-lebeau commented Jul 30, 2024

dd-devflow bot commented Jul 30, 2024

dd-devflow bot commented Jul 30, 2024

BenoitZugmeyer Jul 30, 2024

		getSpy: spyOnProperty(document, 'cookie', 'get').and.callFake(() => cookie),
		setSpy: spyOnProperty(document, 'cookie', 'set').and.callFake((newCookie) => (cookie = newCookie)),

	.filter(
	(configuration) =>
	configuration.sessionName !== 'IE' &&
	// Safari mobile on iOS <= 14.0 does not support
	// the way we flush events on page change
	// TODO check newer version on browserstack
	configuration.sessionName !== 'Safari mobile'
	)

🧪 Update browser matrix for tests #2884

🧪 Update browser matrix for tests #2884

Conversation

thomas-lebeau commented Jul 22, 2024 • edited Loading

Motivation

Changes

Testing

cit-pr-commenter bot commented Jul 22, 2024 • edited Loading

Bundles Sizes Evolution

codecov-commenter commented Jul 24, 2024 • edited Loading

Codecov Report

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

thomas-lebeau commented Jul 26, 2024

dd-devflow bot commented Jul 26, 2024

dd-devflow bot commented Jul 26, 2024

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

dd-devflow bot commented Jul 26, 2024

dd-devflow bot commented Jul 26, 2024

dd-devflow bot commented Jul 26, 2024

dd-devflow bot commented Jul 26, 2024

thomas-lebeau commented Jul 30, 2024

dd-devflow bot commented Jul 30, 2024

dd-devflow bot commented Jul 30, 2024

Choose a reason for hiding this comment

thomas-lebeau commented Jul 22, 2024 •

edited

Loading

cit-pr-commenter bot commented Jul 22, 2024 •

edited

Loading

codecov-commenter commented Jul 24, 2024 •

edited

Loading