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

fix(ci): revert back to xcpretty from xcbeautify #4814

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

Conversation

armcknight
Copy link
Member

#skip-changelog

@armcknight armcknight changed the title Revert "chore(ci): replace xcpretty with xcbeautify (#4645)" fix(ci): revert back to xcpretty from xcbeautify Feb 8, 2025
Comment on lines +6 to -14
rbenv install --skip-existing
rbenv exec gem update bundler
rbenv exec bundle install
clang-format --version | awk '{print $$3}' > scripts/.clang-format-version
swiftlint version > scripts/.swiftlint-version

# installs the tools needed to test various CI tasks locally
init-ci: init
brew bundle --file Brewfile-ci
rbenv install --skip-existing
rbenv exec gem update bundler
rbenv exec bundle install
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only part that wasn't strictly part of reverting the previous changes. I noticed this while looking at the makefile: rbenv stuff should only be done locally, not on ci (see xcode-test.sh and IS_LOCAL_BUILD)

Copy link
Contributor

Choose a reason for hiding this comment

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

Why should rbenv stuff not be done in CI?

Copy link

github-actions bot commented Feb 8, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1225.65 ms 1242.51 ms 16.86 ms
Size 22.31 KiB 780.87 KiB 758.55 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8f397a7 1236.76 ms 1256.76 ms 20.00 ms
2b55154 1231.36 ms 1247.82 ms 16.46 ms
0559a8f 1236.90 ms 1253.46 ms 16.56 ms
dc0db9e 1222.10 ms 1240.90 ms 18.80 ms
e324230 1230.59 ms 1248.20 ms 17.61 ms
533859f 1237.78 ms 1249.76 ms 11.98 ms
98a8c16 1243.33 ms 1257.86 ms 14.53 ms
c9724f9 1199.38 ms 1229.54 ms 30.16 ms
d77a671 1236.63 ms 1250.66 ms 14.03 ms
087a3b3 1220.20 ms 1239.62 ms 19.42 ms

App size

Revision Plain With Sentry Diff
8f397a7 20.76 KiB 420.55 KiB 399.79 KiB
2b55154 22.84 KiB 402.19 KiB 379.34 KiB
0559a8f 21.58 KiB 419.81 KiB 398.22 KiB
dc0db9e 20.76 KiB 419.62 KiB 398.86 KiB
e324230 22.85 KiB 408.87 KiB 386.02 KiB
533859f 22.85 KiB 408.84 KiB 386.00 KiB
98a8c16 20.76 KiB 431.00 KiB 410.24 KiB
c9724f9 20.76 KiB 427.66 KiB 406.90 KiB
d77a671 21.58 KiB 540.04 KiB 518.46 KiB
087a3b3 21.58 KiB 707.43 KiB 685.85 KiB

Copy link

codecov bot commented Feb 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.391%. Comparing base (a03f232) to head (7ce5cb4).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4814       +/-   ##
=============================================
+ Coverage   91.308%   91.391%   +0.083%     
=============================================
  Files          627       627               
  Lines        74570     74569        -1     
  Branches     26177     26807      +630     
=============================================
+ Hits         68089     68150       +61     
+ Misses        6386      6320       -66     
- Partials        95        99        +4     

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 a03f232...7ce5cb4. Read the comment docs.

fi

if [ $RUN_TEST_WITHOUT_BUILDING == true ]; then
set -o pipefail && NSUnbufferedIO=YES xcodebuild \
set -o pipefail && env NSUnbufferedIO=YES && set -o pipefail && xcodebuild \
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
set -o pipefail && env NSUnbufferedIO=YES && set -o pipefail && xcodebuild \
set -o pipefail && env NSUnbufferedIO=YES xcodebuild \

@armcknight armcknight marked this pull request as ready for review February 8, 2025 03:43
@armcknight
Copy link
Member Author

The last CI failures are due to not having the right invocation of xcpretty, which I think should be fixed in #4815, or at least by applying the same strategy in there

@philipphofmann
Copy link
Member

As discussed on Slack, it might be better to stick to xcbeautify because it's maintained, and we rather want to use a maintained tool.

Furthermore, we can fix not finding failing tests with a simple grep and we can disable the verbose GH actions annotations by removing the github-actions renderer of xcbeautify.

@cpisciotta
Copy link

Hey all 👋 I'm Charles – the maintainer of xcbeautify. Just came across this PR, so I was naturally inclined to understand what's motivating a potential reversion to xcpretty. Can you share what pain points you're facing with xcbeautify? I'd be happy to help where I can.

@philipphofmann
Copy link
Member

@cpisciotta, we are currently trying to make it work with #4828. If we have more problems, we will let you know. Thanks a lot for chiming in.

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

Successfully merging this pull request may close these issues.

4 participants