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

test: Fix data deletion e2e tests #28221

Merged
merged 1 commit into from
Oct 31, 2024
Merged

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Oct 31, 2024

Description

The "Delete MetaMetrics Data" e2e tests were recently broken due to a change in CI configuration. The code-under-test was written to always use the environment variable present for the data deletion source ID and endpoint, but the e2e tests wrongly assumed that it would never be present.

The service has been updated to use the fallback values for e2e test builds, even if the environment variable is present.

Open in GitHub Codespaces

Related issues

Fixes CI failure currently on all branches (e.g. https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/108823/workflows/cbc57b89-8647-4219-b413-24df4fdad95c/jobs/4070258 )

This bug was introduced in #24503, but only began causing failures recently when CI configuration was updated with these new environment variables.

Manual testing steps

See that the data deletion e2e tests succeed even with both data deletion environment variables set.

Screenshots/Recordings

N/A

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.

The "Delete MetaMetrics Data" e2e tests were recently broken due to a
change in CI configuration. The code-under-test was written to always
use the environment variable present for the data deletion source ID
and endpoint, but the e2e tests wrongly assumed that it would never be
present.

The service has been updated to use the fallback values for e2e test
builds, even if the environment variable is present.
@Gudahtt Gudahtt marked this pull request as ready for review October 31, 2024 15:24
@Gudahtt Gudahtt requested a review from a team as a code owner October 31, 2024 15:24
@hjetpoluru hjetpoluru self-requested a review October 31, 2024 15:24
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! Thanks for the quick action.

@hjetpoluru hjetpoluru added this pull request to the merge queue Oct 31, 2024
@Gudahtt Gudahtt removed this pull request from the merge queue due to a manual request Oct 31, 2024
@Gudahtt Gudahtt merged commit f301604 into develop Oct 31, 2024
84 of 85 checks passed
@Gudahtt Gudahtt deleted the fix-data-deletion-e2e-tests branch October 31, 2024 15:26
@github-actions github-actions bot locked and limited conversation to collaborators Oct 31, 2024
@metamaskbot metamaskbot added the release-12.7.0 Issue or pull request that will be included in release 12.7.0 label Oct 31, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [2755584]
Page Load Metrics (1677 ± 56 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint37019241607305147
domContentLoaded15021914165511656
load15101926167711656
domInteractive169742209
backgroundConnect76823199
firstReactRender46127792110
getState44812136
initialActions01000
loadScripts10891440121810048
setupStore107723209
uiStartup16932105185713665
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 16 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.7.0 Issue or pull request that will be included in release 12.7.0 team-extension-platform team-wallet-framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants