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

chore(test): add new unit tests after removing exclusions from Sonar config #13031

Draft
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

EtherWizard33
Copy link
Contributor

@EtherWizard33 EtherWizard33 commented Jan 16, 2025

Description

This PR contains the sonar coverage after removing exclusions for the sonar config file

Related issues

Contributes to: https://github.com/MetaMask/MetaMask-planning/issues/3663

Manual testing steps

run the unit tests using yarn test:unit

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.

…code) from NetworkVerificationInfo component
…code that ran when feature flag was off (which was now dead code)
…ture flag, remove code that ran when feature flag was off (which was now dead code)
…ks on feature flag, remove code that ran when feature flag was off (which was now dead code)
…e. Update the test to mock nav, and snapshot expected to be updated too.
…tests.

In more detail, after removal of feature flag and corresponding dead code of ff-off, test now runs on ff-on and required update to show content of accordion before asserting, and update snapshot to match new expected rendering.
…t component default style was changed in main
…r multichain and the deadcode behind it.

In the case of this component, it didn't directly have multichain code in it but was rendering a component which did (NetworkVerificationInfo)
… feature flag for multichain and the deadcode behind it.

It needs this since it uses components than end up using NetworkVerificationInfo which had some code toggled on and off based on the multichain feature flag which is being removed in this PR.
…eature flag was ON.

This check on the feature flag was used in some components to render stuff that should render only when the flag was ON.
… since all multichain-permission code is ff-on now
@EtherWizard33 EtherWizard33 added Run Smoke E2E Triggers smoke e2e on Bitrise team-wallet-ux labels Jan 16, 2025
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.

Copy link
Contributor

github-actions bot commented Jan 16, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: f4eab91
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a5526c28-39f2-4add-aed6-12f692d53272

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@EtherWizard33 EtherWizard33 removed the Run Smoke E2E Triggers smoke e2e on Bitrise label Jan 16, 2025
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 16.66667% with 35 lines in your changes missing coverage. Please review.

Project coverage is 59.94%. Comparing base (faf5340) to head (c32d4a6).
Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
...ts/Views/AccountPermissions/AccountPermissions.tsx 4.76% 19 Missing and 1 partial ⚠️
...nnectMultiSelector/AccountConnectMultiSelector.tsx 0.00% 5 Missing ⚠️
...ettings/PermissionsSettings/PermissionsManager.tsx 0.00% 3 Missing ⚠️
...rmissionsConnected/NetworkPermissionsConnected.tsx 33.33% 2 Missing ⚠️
...etworkVerificationInfo/NetworkVerificationInfo.tsx 50.00% 0 Missing and 1 partial ⚠️
...components/Views/AccountConnect/AccountConnect.tsx 0.00% 1 Missing ⚠️
...rmissionsConnected/AccountPermissionsConnected.tsx 0.00% 1 Missing ⚠️
app/components/Views/BrowserTab/index.js 0.00% 1 Missing ⚠️
app/components/Views/Settings/index.tsx 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13031      +/-   ##
==========================================
+ Coverage   59.83%   59.94%   +0.10%     
==========================================
  Files        1873     1884      +11     
  Lines       42121    42202      +81     
  Branches     5473     5476       +3     
==========================================
+ Hits        25204    25298      +94     
+ Misses      15235    15213      -22     
- Partials     1682     1691       +9     

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

@EtherWizard33 EtherWizard33 changed the title chore: test if all passes test: new unit tests required for new code after the files with unit test coverage exclusion are cleared from the sonar local config file Jan 17, 2025
@EtherWizard33 EtherWizard33 changed the base branch from main to refactor-3663-chain-permissions-feature-flag-etc January 17, 2025 09:38
@EtherWizard33 EtherWizard33 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed e2e-test labels Jan 17, 2025
Copy link
Contributor

github-actions bot commented Jan 17, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: c32d4a6
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/8344858a-1e98-421b-abbd-63b31d1bcabd

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@EtherWizard33 EtherWizard33 changed the title test: new unit tests required for new code after the files with unit test coverage exclusion are cleared from the sonar local config file test: Add new unit tests after removing exclusions from Sonar config Jan 17, 2025
@EtherWizard33 EtherWizard33 changed the title test: Add new unit tests after removing exclusions from Sonar config test: add new unit tests after removing exclusions from Sonar config Jan 17, 2025
@EtherWizard33 EtherWizard33 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Jan 17, 2025
Copy link
Contributor

github-actions bot commented Jan 17, 2025

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 935f02c
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a5f37aee-a15a-4672-aa35-dbe683f4e2c6

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@EtherWizard33 EtherWizard33 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Jan 20, 2025
@EtherWizard33 EtherWizard33 changed the base branch from refactor-3663-chain-permissions-feature-flag-etc to main January 20, 2025 09:30
@EtherWizard33 EtherWizard33 changed the title test: add new unit tests after removing exclusions from Sonar config chore(test): add new unit tests after removing exclusions from Sonar config Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Run Smoke E2E Triggers smoke e2e on Bitrise team-wallet-ux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants