Skip to content

Remove unnecessary assertion in e2e metrics test#9974

Merged
Gudahtt merged 1 commit intodevelopfrom
remove-unnecessary-e2e-metrics-test-assertion
Dec 2, 2020
Merged

Remove unnecessary assertion in e2e metrics test#9974
Gudahtt merged 1 commit intodevelopfrom
remove-unnecessary-e2e-metrics-test-assertion

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Dec 2, 2020

The assertion ensuring that there were at least 3 metrics received didn't end up being useful. If this assertion fails, it doesn't explain what segment events were received.

By removing this assertion and letting the later assertions catch this case, we at least learn which of the three expected events were present.

The assertion ensuring that there were at least 3 metrics received
didn't end up being useful. If this assertion fails, it doesn't explain
what segment events _were_ received.

By removing this assertion and letting the later assertions catch this
case, we at least learn which of the three expected events were
present.
@Gudahtt Gudahtt marked this pull request as ready for review December 2, 2020 20:39
@Gudahtt Gudahtt requested a review from a team as a code owner December 2, 2020 20:39
@Gudahtt Gudahtt requested a review from rekmarks December 2, 2020 20:39
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [3b66280]
Page Load Metrics (490 ± 60 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint31107582512
domContentLoaded30767448812460
load30867649012460
domInteractive30767448812460

Copy link
Copy Markdown
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

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

LGTM!

@Gudahtt Gudahtt merged commit ba98edf into develop Dec 2, 2020
@Gudahtt Gudahtt deleted the remove-unnecessary-e2e-metrics-test-assertion branch December 2, 2020 20:57
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants