Skip to content

[Test] Skip Mirror.swift's Subclass, Fields, and Cases tests on older OSes. #26881

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 1 commit into from
Aug 28, 2019
Merged

[Test] Skip Mirror.swift's Subclass, Fields, and Cases tests on older OSes. #26881

merged 1 commit into from
Aug 28, 2019

Conversation

mikeash
Copy link
Contributor

@mikeash mikeash commented Aug 27, 2019

rdar://problem/54754304

@mikeash mikeash requested a review from jrose-apple August 27, 2019 21:46
@mikeash
Copy link
Contributor Author

mikeash commented Aug 27, 2019

@jrose-apple Can you confirm that skipping these tests on older OSes is the right thing to do?

@mikeash
Copy link
Contributor Author

mikeash commented Aug 27, 2019

@swift-ci please test

@jrose-apple
Copy link
Contributor

I would prefer #available to .skip because it's more obvious that we're missing the abstraction we really need, but yes, these sadly won't work on backwards-deployment targets. cc @jckarter

@mikeash
Copy link
Contributor Author

mikeash commented Aug 28, 2019

Thanks for confirming. I'll change it to #available.

@mikeash mikeash force-pushed the skip-new-mirror-tests-on-old-oses branch from 10888f1 to 0345186 Compare August 28, 2019 13:57
@mikeash
Copy link
Contributor Author

mikeash commented Aug 28, 2019

@swift-ci please test and merge

@jckarter
Copy link
Contributor

I don't have the background on what the runtime-version-dependent change is here, sorry @jrose-apple. What changed in Mirror?

@jrose-apple
Copy link
Contributor

Sorry, this was #25984.

@mikeash
Copy link
Contributor Author

mikeash commented Aug 28, 2019

@swift-ci please test and merge

@swiftlang swiftlang deleted a comment Aug 28, 2019
@swiftlang swiftlang deleted a comment Aug 28, 2019
@jckarter
Copy link
Contributor

Thanks @jrose-apple. Using #available makes sense to me too. Would it be worth introducing an expected failure on the else case, so that if we manage to back-deploy demangler fixes in the future, we're reminded to update this test to remove the conditionals?

@swift-ci swift-ci merged commit 8f56771 into swiftlang:master Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrose-apple jrose-apple Awaiting requested review from jrose-apple

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mikeash @jrose-apple @jckarter @swift-ci