fix: attribution link #25947
fix: attribution link #25947
Conversation
|
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 [2c27d77]
Page Load Metrics (265 ± 268 ms)
Bundle size diffs
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #25947 +/- ##
===========================================
+ Coverage 69.67% 69.69% +0.02%
===========================================
Files 1400 1401 +1
Lines 49513 49577 +64
Branches 13692 13701 +9
===========================================
+ Hits 34495 34548 +53
- Misses 15018 15029 +11 ☔ View full report in Codecov by Sentry. |
| @@ -18,8 +18,7 @@ const supportContactUs = | |||
| const mmiHomePage = 'https://metamask.io/institutions/'; | |||
| const privacyAndNotice = 'https://consensys.io/privacy-notice'; | |||
| const openSeaTermsOfUse = 'https://opensea.io/securityproviderterms'; | |||
| const metamaskAttributions = | |||
| 'https://raw.githubusercontent.com/MetaMask/metamask-extension/develop/attribution.txt'; | |||
| const metamaskAttributions = `https://raw.githubusercontent.com/MetaMask/metamask-extension/v${process.env.METAMASK_VERSION}/attribution.txt`; | |||
There was a problem hiding this comment.
Similarly here, I am wondering if process.env.METAMASK_VERSION is set to anything
There was a problem hiding this comment.
Compared to the previous unit test, this is an end-to-end test where the extension is built and installed on the browser. During the build process, the version is assigned from the manifest. I tried running the e2e tests locally; although they did not complete successfully, the extension was installed, and the wallet was recovered. However, the process stopped at the password setup, which I completed manually. When I checked, the attribution from the settings they were using the correct link.
Also in the current pipeline I see that mmi e2e are skipped as I made explicitly this commit to make the pipeline fail (53b811f) but it succeeded
There was a problem hiding this comment.
During the build process, the version is assigned from the manifest
Yes, but the build process is separate from test processes. The this file is run as part of the test process, where it's not set.
There was a problem hiding this comment.
Yes, you're right. I was able to run the tests locally, and they failed. In my previous message, I mentioned that it's working because I only checked that the extension had the correct attribution link, which it does. However, the test was expecting https://raw.githubusercontent.com/MetaMask/metamask-extension/vundefined/attribution.txt, but it received https://raw.githubusercontent.com/MetaMask/metamask-extension/v11.16.15-mmi.0/attribution.txt.
There was a problem hiding this comment.
I had to remove the test because, unfortunately, the build and test processes are separate. We can't reference the version used during the build in the test. The change has been approved by @zone-live
There was a problem hiding this comment.
Hmm, we could read package.json from within the test, and populate it that way. But removing it sounds OK to me too.
Builds ready [b78ba46]
Page Load Metrics (141 ± 169 ms)
Bundle size diffs
|
Builds ready [619d425]
Page Load Metrics (166 ± 182 ms)
Bundle size diffs
|
Builds ready [53b811f]
Page Load Metrics (206 ± 209 ms)
Bundle size diffs
|
|
MMI e2e attribution link check has been removed as it can't be dynamically updated |
|
Builds ready [b80a35e]
Page Load Metrics (353 ± 303 ms)
Bundle size diffs
|
metamaskbot
added
the
release-12.3.0
Description
Update the attribution link to match the current metamask version
Related issues
Fixes: #2655
Manual testing steps
GIVEN the user has already completed setup
WHEN they access Settings>About>Attributions
THEN the user should be redirected to the RC link for attributions
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist