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.

Sign up for GitHub

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

Jump to bottom

feat: 10550 Re-introduce test for measuring cold app start + JS bundle load time #11387

Merged
merged 48 commits into from
Sep 30, 2024
Merged

feat: 10550 Re-introduce test for measuring cold app start + JS bundle load time #11387

merged 48 commits into from
Sep 30, 2024

Conversation

MarioAslau
Copy link
Contributor

Description

Using react-native-performance as a performance measurement tool, which will be implemented here - #10521, we want to utilize E2E world for measuring cold app start + JS bundle load time.

Technical Details

  • Add E2E test that measures The new App Start time in ColdStartToOnboardingScreen.feature replacing the old test
  • Added a baseline measure to compare App Start time against.
    Values from 10 tests:
    //test 1: 383
    //test 2: 334
    //test 3: 380
    //test 4: 410
    //test 5: 394
    //test 6: 352
    //test 7: 602
    //test 8: 364
    //test 9: 359
    //test 10: 438
  • E2E test check the performance numbers against the baseline value. If the new performance numbers is higher than the threshold, fail the test and vice versa

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

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.

@MarioAslau MarioAslau self-assigned this Sep 23, 2024
@github-actions GitHub Actions
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.

@MarioAslau MarioAslau marked this pull request as ready for review September 23, 2024 19:40
@MarioAslau MarioAslau requested a review from a team as a code owner September 23, 2024 19:40
cortisiko
cortisiko previously approved these changes Sep 23, 2024
View reviewed changes
Copy link
Member

@cortisiko cortisiko left a comment

Choose a reason for hiding this comment

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

🚀 🚀 🚀

Cal-L
Cal-L reviewed Sep 26, 2024
View reviewed changes
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

Fix lint issues

@Cal-L Cal-L added the Run Smoke E2E Triggers smoke e2e on Bitrise label Sep 27, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 27, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: da77c6e
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/0fbcdff2-68de-4842-b7b0-683fd89b122b

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

@MarioAslau MarioAslau removed the Run Smoke E2E Triggers smoke e2e on Bitrise label Sep 30, 2024
tommasini
tommasini approved these changes Sep 30, 2024
View reviewed changes
Copy link
Contributor

@tommasini tommasini left a comment

Choose a reason for hiding this comment

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

LGTM! Awesome!

app/components/Views/OnboardingCarousel/index.tsx Show resolved Hide resolved
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
81.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@MarioAslau MarioAslau added this pull request to the merge queue Sep 30, 2024
Merged via the queue into main with commit 694cb49 Sep 30, 2024
39 checks passed
@MarioAslau MarioAslau deleted the feat/10550-reintroduce-coldAppStart-test branch September 30, 2024 11:39
@github-actions github-actions bot locked and limited conversation to collaborators Sep 30, 2024
@tommasini tommasini mentioned this pull request Sep 30, 2024
Merged
7 tasks
@MarioAslau MarioAslau restored the feat/10550-reintroduce-coldAppStart-test branch September 30, 2024 15:47
@MarioAslau MarioAslau added the Run Smoke E2E Triggers smoke e2e on Bitrise label Sep 30, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 30, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 281f102
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/1bcd78c1-fd69-4714-b9e1-b8a52d0814da

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

@github-actions github-actions bot unlocked this conversation Sep 30, 2024
github-merge-queue bot pushed a commit that referenced this pull request Sep 30, 2024
@tommasini
chore: revert measuring with react native performance (#11504)
98f656f 
## **Description**

Revert of this PR:
#11387

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.
@gauthierpetetin gauthierpetetin added the team-tiger Tiger team (for tech debt reduction + performance improvements) label Oct 1, 2024
@gauthierpetetin gauthierpetetin added the release-7.33.0 Issue or pull request that will be included in release 7.33.0 label Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Cal-L Cal-L Cal-L left review comments

@tommasini tommasini tommasini approved these changes

@sethkfman sethkfman Awaiting requested review from sethkfman

@cortisiko cortisiko Awaiting requested review from cortisiko

Assignees

@MarioAslau MarioAslau

Labels
No QA Needed Apply this label when your PR does not need any QA effort. release-7.33.0 Issue or pull request that will be included in release 7.33.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-mobile-platform team-tiger Tiger team (for tech debt reduction + performance improvements)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MarioAslau @Cal-L @cortisiko @tommasini @gauthierpetetin