-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: flag Confirmations tests as Smoke tests #7710
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. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7710 +/- ##
=======================================
Coverage 36.60% 36.60%
=======================================
Files 1092 1092
Lines 29178 29178
Branches 2678 2678
=======================================
Hits 10681 10681
Misses 17885 17885
Partials 612 612 ☔ View full report in Codecov by Sentry. |
E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/e7e6164a-e4e8-4cce-8a08-5426e78286f3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the type-sign test from the smoke tag? It takes 5 mins for that one spec file to run compared to the other tests which take ~1 minute.
thanks for your comment @cortisiko . It's true that typed signature spec took a bit longer. We've re-arranged the tests so we can benefit from the parallelization and increase the speed for those. |
@seaona amazing, thanks for splitting the tests.
Actually, the nightly regression test build (which includes these signature tests) caught the failing tests. They should be fixed in main now. Feel free to pull in main and rerun the tests. |
After the test signatures change, all spec files are <= 2mins. Run #7 - after Signatures Change - Detox Build & Test
Run #8 - after Signatures Change - Detox Build & Test
Run #9 - after Signatures Change - Detox Build & Test
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR looks good. Nit: Can you put the sign tests in a signatures folder?
Kudos, SonarCloud Quality Gate passed! |
thank you @cortisiko done |
Description
In this PR we are flagging all the Confirmations tests as Smoke test. This includes the following spec files:
Notice the amount of time added on the runs is not significant compared to the amount of scenarios we add, giving everyone more confidence on making changes in the Wallet. See numbers below.
Notice the tests are stable, being 100% Successful after several ci runs. This was achieved thanks to this fix by @vinistevam
Recent examples how Confirmations tests captured an error on
main
Here is an example of how running all the Confirmations tests, would have captured and prevent an error in
main
. Luckily we were running them locally and could spot the error and connect the dots with the slack thread.Here Signatures flow broken in main, that would have been captured by Confirmations tests if run as Smoke https://app.bitrise.io/build/fa65a941-6cd6-4b0c-9ece-151e76cdbb0d
Test Runs Data
Run #1 - Detox Build & Test
Run #2 - Detox Build & Test
Run #3 - Detox Build & Test
Run #4 - Detox Build & Test
Run #5 - Detox Build & Test
Related issues
Fixes: #
Manual testing steps
Trigger ci test runs from Bitrise and see that all tests are passing.
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist