Skip to content

chore: upgrade detox #95

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

Merged

Conversation

chakrihacker
Copy link
Contributor

@chakrihacker chakrihacker commented May 1, 2019

Upgrade detox:

Upgrade detox for xcode 10 and above support

closes #94

@tido64 tido64 added platform: iOS This is iOS specific bug Something isn't working labels May 1, 2019
@chakrihacker
Copy link
Contributor Author

Android now uses android test implementation
https://github.com/wix/Detox/blob/master/docs/Guide.Migration.md
Followed above guide to migrate versions

Copy link
Member

@krizzu krizzu left a comment

Choose a reason for hiding this comment

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

Thanks for bringing this up 🥇

I left few comments, mind having a look?

Quoting:
Detox 12 has been released with Xcode 10.2 support and minimum Xcode version of 10.1.

This mean a breaking change for someone who did not upgrade their Xcode. So here's a real deal: should we merge this PR and put a note about Xcode support or hold for some time before doing that transition?

CC @tido64

thanks.

@@ -117,8 +117,7 @@ android {
versionCode 1
versionName "1.0"
testBuildType System.getProperty('testBuildType', 'debug')
missingDimensionStrategy "minReactNative", "minReactNative46"
testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner"
testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason to move to androidx? I haven't seen anything in Detox's migration guide that mention it. I've seen (and had) a bad experience with mixing support libs with androidx in one project, so ideally we'd love to avoid that conflict.

Copy link
Contributor Author

@chakrihacker chakrihacker May 3, 2019

Choose a reason for hiding this comment

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

When migrated from 10.x to 11.x androidx is necessary for detox

@tido64
Copy link
Member

tido64 commented May 3, 2019

@krizzu: Regarding Xcode, one should be on at least 10.1 nowadays to build for latest iOS. But isn't this just if you need to compile the tests? Regular consumers should not be affected, correct?

@chakrihacker
Copy link
Contributor Author

chakrihacker commented May 3, 2019

as @tido64 mentioned, Xcode 10.1 is necessary only for contributors, it doesn't effect users

Detox 12 released with Xcode 10 support, but we upgraded from 9 to 12 which has more breaking changes

@krizzu krizzu changed the title Upgrade detox chore: upgrade detox May 8, 2019
@krizzu krizzu merged commit 9fc5f28 into react-native-async-storage:master May 8, 2019
@krizzu
Copy link
Member

krizzu commented May 10, 2019

🎉 This PR is included in version 1.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working platform: iOS This is iOS specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Current detox version doesn't support Xcode 10.2
3 participants