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 not running tests below iOS 14 #919

Merged
merged 2 commits into from
Mar 26, 2021

Conversation

nuno-vieira
Copy link
Member

@nuno-vieira nuno-vieira commented Mar 25, 2021

In This PR

The tests were not running below < iOS 14. It would fall back to the default device 🤦‍♂️

Issues found:

  • The Xcode version was hardcoded, and it can't be because each Xcode includes different default ios simulator runtimes.
  • There was missing a pipe "|" for the action to actually work -.-
  • The stress tests were not even running on the specified versions because it was missing the options[:device] parameter on the lanes. Even if the prepare-simulator-action was working, the stress tests were always running on the default device.

The script needs to have a condition because of how the Xcode path is different on 10.3 vs >10.3
More details here: actions/runner-images#551 (comment)

Now I finally got why the iOS 12 coverage was the same as the iOS 14 😄

It probably would be a good idea to open source this action actually 🤔 It took a lot of time to make it work properly for every case.

@nuno-vieira nuno-vieira added the 🏠 Meta A PR that updates repo or project information label Mar 25, 2021
@nuno-vieira nuno-vieira requested a review from VojtaStavik March 25, 2021 19:25
@codecov
Copy link

codecov bot commented Mar 25, 2021

Codecov Report

Merging #919 (7b897d7) into main (7d4c9dd) will increase coverage by 0.18%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #919      +/-   ##
==========================================
+ Coverage   89.17%   89.36%   +0.18%     
==========================================
  Files         201      201              
  Lines        8271     8271              
==========================================
+ Hits         7376     7391      +15     
+ Misses        895      880      -15     
Flag Coverage Δ
llc-tests 89.36% <ø> (+0.18%) ⬆️
llc-tests-ios12 85.38% <ø> (-3.80%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ources/StreamChat/Database/DatabaseContainer.swift 96.80% <0.00%> (+0.79%) ⬆️
.../Utils/InternetConnection/InternetConnection.swift 54.95% <0.00%> (+0.90%) ⬆️
...ocketClient/Engine/StarscreamWebSocketEngine.swift 30.95% <0.00%> (+30.95%) ⬆️

Continue to review full report at Codecov.

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

@nuno-vieira nuno-vieira marked this pull request as draft March 25, 2021 19:43
@nuno-vieira nuno-vieira force-pushed the fix-ci-not-running-tests-on-ios-below-14 branch 26 times, most recently from 3729695 to b82af52 Compare March 26, 2021 01:24
@nuno-vieira nuno-vieira force-pushed the fix-ci-not-running-tests-on-ios-below-14 branch 6 times, most recently from cb03b92 to 7b897d7 Compare March 26, 2021 02:03
@nuno-vieira nuno-vieira marked this pull request as ready for review March 26, 2021 02:31
Copy link
Contributor

@VojtaStavik VojtaStavik left a comment

Choose a reason for hiding this comment

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

This is cool 👍 🎉 Thank you

@nuno-vieira nuno-vieira merged commit 35ba8ca into main Mar 26, 2021
@nuno-vieira nuno-vieira deleted the fix-ci-not-running-tests-on-ios-below-14 branch March 26, 2021 10:50
Copy link
Contributor

@DominikBucher12 DominikBucher12 left a comment

Choose a reason for hiding this comment

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

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏠 Meta A PR that updates repo or project information
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants