Skip to content

Make e2e test results available in Bitrise #6136

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
Merged
merged 10 commits into from
Apr 7, 2023
Merged

Make e2e test results available in Bitrise #6136

merged 10 commits into from
Apr 7, 2023
+64 −23

Conversation

@NicolasMassart
Copy link
Contributor

@NicolasMassart NicolasMassart commented Apr 6, 2023 •
edited
Loading

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add needs-qa label when dev review is completed
  5. Add QA Passed label when QA has signed off

Description

Update existing bitrise upload steps and make the Android test results always uploaded.
Also add reporter to generate test on iOS e2e and upload.

  1. What is the reason for the change?

    • Test were not pushed to Bitrise test tab on failure (and not even on success by the way)

  2. What is the improvement/solution?

    • update upload to Bitrise step version as v1 is not compatible anymore (leads to not uploading anything)
    • update android wdio workflow to always publish test reports, even on failure
    • add custom reporter in Detox config
    • add Jest Junit reporter as a dev dependency for the custom Detox reporter
    • add custom-test-results-export@1 steps in iOS e2e workflow to search for reports and upload them. The default copy script was not working.

Issue

fixes MetaMask/mobile-planning#733

Testing

Android e2e (Appium/wdio)

  • One Android test ran on my own Bitrise setup but given the amount time required to run tests, I haven't ran them again on Metamask Bitrise. But it works. Feel free to run a test if you want, they fail for now, so this would require to run only a simple one that passes (making Android e2e pass is clearly not part of this PR).
    image

iOS e2e (Detox)

iOS e2e are not passing because of some deeplink issues, so I ran them with only individual test because I only needed to check the report part:

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2023

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.

@socket-security
Copy link

socket-security bot commented Apr 6, 2023

New dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

Pull request alert summary
Issue Status
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues

📊 Modified Dependency Overview:

➕ Added Package Capability Access +/- Transitive Count Publisher
jest-junit@15.0.0 None +1 jsonp

@NicolasMassart
removed spec test
ad1e8f8 
debug report ios

debug report ios

debug report ios

debug report ios

debug report ios

debug report ios

debug report ios

debug report ios

move working code to ios e2e workflow

delete test report

remove debug and useless param

update lock file
@NicolasMassart NicolasMassart force-pushed the 733_e2e_test_results_available_in_bitrise branch from 8a8d5ee to ad1e8f8 Compare April 6, 2023 11:30
@NicolasMassart NicolasMassart added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) needs-qa Any New Features that needs a full manual QA prior to being added to a release. and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Apr 6, 2023
@NicolasMassart NicolasMassart marked this pull request as ready for review April 6, 2023 17:14
@NicolasMassart NicolasMassart requested a review from a team as a code owner April 6, 2023 17:14
sethkfman
sethkfman approved these changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@sethkfman sethkfman left a comment

Choose a reason for hiding this comment

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

LGTM

@cortisiko cortisiko removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Apr 6, 2023
cortisiko
cortisiko approved these changes Apr 6, 2023
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.

ty @NicolasMassart !! 🌮 🌮 🌮

@sethkfman sethkfman added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Apr 6, 2023
@cortisiko
Copy link
Member

cortisiko commented Apr 6, 2023 •
edited
Loading

alright @NicolasMassart I was unable to generate the test report on a failed test build: https://app.bitrise.io/build/8437ed12-96ac-4303-aa1f-283ef9cb9b03

I get this:

Collecting test results...
No test results found

You can test this the quick and easy way by using the following env variables such as BROWSERSTACK_TAG_EXPRESSION and BROWSERSTACK_APP_URL. You can read up more here

Ideally when you kick off the e2e build, you want to pass in the browser stack app url as well as the tag you want to run. The tag with one 1 test is @Performance so you should probably use that one.

So to be clear to generate the test results quickly on a test build:

  • in bitrise go to the advanced section when you are about to kick off a build
  • scroll to the Custom Environment Variables section
  • set the BROWSERSTACK_APP_URL env var to: bs://b8f0203f25deef45e24fa288a10592c6a4daa7b3
  • the BROWSERSTACK_TAG_EXPRESSION to: @Performance

then the work flow you should use is: wdio_android_e2e_test

@cortisiko cortisiko added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Apr 6, 2023
Fatxx
Fatxx reviewed Apr 6, 2023
View reviewed changes
@sethkfman sethkfman added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Apr 7, 2023
@cortisiko cortisiko added QA Passed QA testing has been completed and passed and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Apr 7, 2023
@cortisiko cortisiko merged commit 649ced3 into main Apr 7, 2023
@cortisiko cortisiko deleted the 733_e2e_test_results_available_in_bitrise branch April 7, 2023 22:44
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sethkfman sethkfman sethkfman approved these changes

@cortisiko cortisiko cortisiko approved these changes

+1 more reviewer

@Fatxx Fatxx Fatxx left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bitrise QA Passed QA testing has been completed and passed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@NicolasMassart @cortisiko @Fatxx @sethkfman