Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Adding state per window in e2e, excluding null state #25900

Merged
merged 2 commits into from
Jul 19, 2024

Conversation

vthomas13
Copy link
Contributor

@vthomas13 vthomas13 commented Jul 17, 2024

A suggestion from @hjetpoluru, this PR aims to add better context to artifacts by logging state from multiple windows, as well as removing state artifacts that are null. This is similar to #25453.
Also fixes an mv3 offscreen issue found by @HowardBraham. The runner would get hung up on taking a screenshot if it was the offscreen window.

Description

Open in GitHub Codespaces

Related issues

Fixes: #25874

Manual testing steps

  1. Run a failing e2e test
  2. Verify that the test-failure-state.json artifact are now numbered and the number matches the relevant screenshot

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@vthomas13 vthomas13 requested a review from a team as a code owner July 17, 2024 14:21
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@vthomas13 vthomas13 changed the title Adding state per window in e2e, excluding null state feat: Adding state per window in e2e, excluding null state Jul 17, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [d975290]
Page Load Metrics (262 ± 253 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6459214312661
domContentLoaded9114332813
load392064262527253
domInteractive9114332813
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

hjetpoluru
hjetpoluru previously approved these changes Jul 17, 2024
Copy link
Contributor

@hjetpoluru hjetpoluru left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@chloeYue chloeYue left a comment

Choose a reason for hiding this comment

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

LGTM !

@metamaskbot
Copy link
Collaborator

Builds ready [aa65cda]
Page Load Metrics (247 ± 254 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint672341083517
domContentLoaded96729189
load431953247528254
domInteractive96728189
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@vthomas13 vthomas13 force-pushed the null-state-failing-e2e branch from aa65cda to 6f58512 Compare July 19, 2024 17:22
Copy link

sonarcloud bot commented Jul 19, 2024

Copy link
Contributor

@hjetpoluru hjetpoluru left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

codecov bot commented Jul 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.69%. Comparing base (ee1dfe4) to head (6f58512).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #25900   +/-   ##
========================================
  Coverage    69.69%   69.69%           
========================================
  Files         1401     1401           
  Lines        49593    49593           
  Branches     13708    13708           
========================================
  Hits         34563    34563           
  Misses       15030    15030           

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

@hjetpoluru hjetpoluru merged commit 38f2d6b into develop Jul 19, 2024
78 of 79 checks passed
@hjetpoluru hjetpoluru deleted the null-state-failing-e2e branch July 19, 2024 18:33
@github-actions github-actions bot locked and limited conversation to collaborators Jul 19, 2024
@metamaskbot metamaskbot added the release-12.3.0 Issue or pull request that will be included in release 12.3.0 label Jul 19, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [6f58512]
Page Load Metrics (81 ± 21 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint762991115727
domContentLoaded9170333316
load41220814521
domInteractive9170333316
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.3.0 Issue or pull request that will be included in release 12.3.0 team-extension-platform
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Json state link in the artifacts is showing null for multiple screens
4 participants