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.

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

Add support for precondition testing on Apple Silicon #948

Merged
merged 4 commits into from
Dec 1, 2021
Merged

Add support for precondition testing on Apple Silicon #948

merged 4 commits into from
Dec 1, 2021
+108 −74

Conversation

svenmuennich
Copy link
Contributor

This updates the dependency mattgallagher/CwlPreconditionTesting to v2.1.0 and thereby fixes precondition testing for Macs running on Apple Silicon.

Closes #851

@svenmuennich
Update mattgallagher/CwlPreconditionTesting to v2.1.0

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
dfa2e0f
@svenmuennich svenmuennich mentioned this pull request Nov 29, 2021
Closed
1 task
SebastianOsinski
SebastianOsinski reviewed Nov 30, 2021
View reviewed changes
Copy link

@SebastianOsinski SebastianOsinski left a comment

Choose a reason for hiding this comment

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

Shouldn't also ThrowAssertion.swift be updated? It contains explicit x86_64 architecture check.

@svenmuennich
Copy link
Contributor Author

Shouldn't also ThrowAssertion.swift be updated? It contains explicit x86_64 architecture check.

You're absolutely right! I fixed this and verified that the tests are executed on arm64 😅

@svenmuennich svenmuennich force-pushed the svenmuennich/update-cwlpreconditiontesting-for-apple-silicon branch from 3c7a9e8 to f6b1429 Compare November 30, 2021 09:42
@svenmuennich
Copy link
Contributor Author

Thanks for running the CI. I noticed that the library update added an import that looks like it was deliberately removed in this project. I remove it again and running the test from the Xcode project works now.

Could you please trigger the CI again @ikesyo?

SebastianOsinski
SebastianOsinski reviewed Nov 30, 2021
View reviewed changes
...kouts/CwlPreconditionTesting/Sources/CwlPreconditionTesting/CwlBadInstructionException.swift Outdated

import Foundation

#if SWIFT_PACKAGE
#if SWIFT_PACKAGE || COCOAPODS
Copy link

@SebastianOsinski SebastianOsinski Nov 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Unfortunately, because of that change (and the same one in other file), Nimble does not compile anymore when used with CocoaPods in the project.

@svenmuennich
Copy link
Contributor Author

Alright, next round. ./test podspec passes locally now.

ikesyo
ikesyo approved these changes Dec 1, 2021
View reviewed changes
Copy link
Member

@ikesyo ikesyo left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution ✨

@ikesyo ikesyo merged commit 0bf627c into Quick:main Dec 1, 2021
@svenmuennich svenmuennich deleted the svenmuennich/update-cwlpreconditiontesting-for-apple-silicon branch December 1, 2021 16:36
@rafaelnobrekz
Copy link

Hello! Thanks for merging this! Any chance there will be a release/podspec update soon ? I'm pointing to git/main right now and it runs great on Apple Silicon!

@acecilia
Copy link
Contributor

acecilia commented Jan 7, 2022

@ikesyo any chance to get a new release? 🙏

@BalestraPatrick BalestraPatrick mentioned this pull request Jan 26, 2022
Merged
@acecilia acecilia mentioned this pull request Mar 4, 2022
Closed
1 task
acecilia added a commit to revolut-mobile/Nimble that referenced this pull request Apr 13, 2022
@acecilia
Merge pull request Quick#948 from pickware/svenmuennich/update-cwlpre…
632676e 
…conditiontesting-for-apple-silicon
@jessesquires jessesquires added this to the v10.0.0 milestone Apr 15, 2022
@stonko1994 stonko1994 mentioned this pull request Nov 2, 2023
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SebastianOsinski SebastianOsinski

@ikesyo ikesyo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v10.0.0
Development

Successfully merging this pull request may close these issues.

Trouble with testing fatalError()
6 participants
@svenmuennich @rafaelnobrekz @acecilia @ikesyo @SebastianOsinski @jessesquires