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(ci): refactor xcode-test from positional to named arguments #4808

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

philprime
Copy link
Contributor

@philprime philprime commented Feb 6, 2025

Changes the scripts/xcode-test.sh to use named arguments instead of positional.

This should improve usability of the script, and reduce accidental usage errors when changing the script.

Furthermore it surfaced that the 7th argument ${{ matrix.scheme }} is actually used as the DERIVED_DATA path, instead of the 8th argument

run: ./scripts/xcode-test.sh ${{matrix.platform}} ${{matrix.test-destination-os}} $GITHUB_REF_NAME build-for-testing "${{matrix.device}}" TestCI ${{matrix.scheme}}

PLATFORM="${1}"
OS=${2:-latest}
REF_NAME="${3-HEAD}"
COMMAND="${4:-test}"
DEVICE=${5:-iPhone 14}
CONFIGURATION_OVERRIDE="${6:-}"
DERIVED_DATA_PATH="${7:-}"
TEST_SCHEME="${8:-Sentry}"

#skip-changelog

@philprime philprime force-pushed the philprime/refactor-xcode-test branch from 5946f1b to 12b0300 Compare February 6, 2025 11:33
@philprime philprime marked this pull request as ready for review February 6, 2025 11:47
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.376%. Comparing base (bdd896e) to head (2eaabb7).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4808       +/-   ##
=============================================
- Coverage   91.406%   91.376%   -0.031%     
=============================================
  Files          627       627               
  Lines        74546     74530       -16     
  Branches     26799     26738       -61     
=============================================
- Hits         68140     68103       -37     
- Misses        6310      6328       +18     
- Partials        96        99        +3     

see 17 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdd896e...2eaabb7. Read the comment docs.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM.

@philprime philprime self-assigned this Feb 6, 2025
@philprime
Copy link
Contributor Author

Blocked by #4809

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Blocked
Development

Successfully merging this pull request may close these issues.

2 participants