test: add coverage for Security Alerts API fallback (PPOM local) #28565
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
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. |
Builds ready [d70034a]
Page Load Metrics (1754 ± 48 ms)
Bundle size diffs
|
| @@ -48,7 +50,7 @@ async function mockInfuraWithMaliciousResponses(mockServer) { | |||
|
|
|||
| describe('PPOM Blockaid Alert - Multiple Networks Support @no-mmi', function () { | |||
| // eslint-disable-next-line mocha/no-skipped-tests | |||
| it.skip('should show banner alert after switchinig to another supported network', async function () { | |||
| it.skip('should show banner alert after switching to another supported network', async function () { | |||
There was a problem hiding this comment.
Maybe reenable this test?
Builds ready [06c5c9c]
Page Load Metrics (1854 ± 93 ms)
Bundle size diffs
|
pedronfigueiredo
previously approved these changes
Nov 25, 2024
Builds ready [5d6baf0]
Page Load Metrics (2506 ± 133 ms)
Bundle size diffs
|
vinistevam
force-pushed
the
test/add-coverage-fallback-tests-security-alerts-api
branch
from
ff4de5b to
5c57a03
Compare
Builds ready [5c57a03]
Page Load Metrics (1537 ± 50 ms)
Bundle size diffs
|
Builds ready [2698c02]
Page Load Metrics (1815 ± 91 ms)
Bundle size diffs
|
Builds ready [5f18b49]
Page Load Metrics (2176 ± 105 ms)
Bundle size diffs
|
pedronfigueiredo
previously approved these changes
Dec 9, 2024
matthewwalsh0
requested changes
Dec 12, 2024
|
|
||
| async ({ driver }) => { | ||
| await unlockWallet(driver); | ||
| await openDapp(driver); |
There was a problem hiding this comment.
If we're adding new E2E tests, should we be using page object models or creating new ones?
There was a problem hiding this comment.
Good point, I've changed the new tests to use the page object model approach.
| await mockRequest(mockServer, maliciousTradeAlert); | ||
| } | ||
|
|
||
| describe('Security Alerts API - Set Trade farming order @no-mmi', function () { |
There was a problem hiding this comment.
As many of these files have a single test in, would it be simpler to keep them in a single file to reduce the boilerplate overhead?
The duplication and final line count should be minimal if we rely on page object models to abstract the interactions?
There was a problem hiding this comment.
I've aggregated the new tests to reduce the number of files and boilerplate.
Builds ready [4db2f73]
Page Load Metrics (1622 ± 58 ms)
Bundle size diffs
|
vinistevam
force-pushed
the
test/add-coverage-fallback-tests-security-alerts-api
branch
from
45eb8c3 to
4aaaa8b
Compare
Builds ready [368489a]
Page Load Metrics (1841 ± 83 ms)
|
jpuri
reviewed
Jan 8, 2025
| ], | ||
| }) | ||
| .thenJson(201, response); | ||
| } |
There was a problem hiding this comment.
It should be possible to create a common function to avoid code repetition with method mockRequest .
There was a problem hiding this comment.
Good point, added here.
jpuri
reviewed
Jan 8, 2025
|
|
||
| async function mockRequest(server, response) { | ||
| await server | ||
| .forPost(`${SECURITY_ALERTS_PROD_API_BASE_URL}/validate/0x1`) |
There was a problem hiding this comment.
I guess we already have these tests implemented for security alerts api also.
jpuri
previously approved these changes
Jan 8, 2025
Builds ready [49b8d41]
Page Load Metrics (1713 ± 134 ms)
Bundle size diffs
|
Builds ready [e9b1657]
Page Load Metrics (1832 ± 130 ms)
Bundle size diffs
|
OGPoyraz
previously approved these changes
Jan 15, 2025
Builds ready [53ce817]
Page Load Metrics (1817 ± 70 ms)
Bundle size diffs
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR enhances test coverage for the Security Alerts API by reusing the existing PPOM end-to-end (E2E) tests to validate the fallback mechanism. It also introduces a new equivalent E2E test in a dedicated folder to cover the API.
Key Changes
Related issues
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3656
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist