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

Added testBinaryPath as an optional config parameter #1007

Merged
merged 8 commits into from
Nov 11, 2018

Conversation

reime005
Copy link
Contributor

At a specific point in an app configuration the 'testBinaryPath' was not correct and I had the need to configure it manually. Please let me know if that does not make sense.

Tested on Android emulator, with and without the testBinaryPath parameter.

@reime005 reime005 requested a review from rotemmiz as a code owner October 29, 2018 21:09
@chips5k
Copy link

chips5k commented Oct 30, 2018

+1 for this - my team need this feature as the conventions guessed path is completely wrong in our setup. the only work around at present is to rename and shift our files to match what its expecting...

@rotemmiz
Copy link
Member

rotemmiz commented Nov 1, 2018

Thanks for this PR!
While the PR code is good, I have an issue with the documentation.

  1. Since it's optional, I think testBinaryPath should be removed from the introduction example.
  2. The documentation should also note this is android-only config.
  3. Add testBinaryPath to https://github.com/wix/Detox/blob/master/docs/APIRef.Configuration.md#device-configuration

@reime005
Copy link
Contributor Author

reime005 commented Nov 1, 2018

sure, done

@rotemmiz
Copy link
Member

rotemmiz commented Nov 9, 2018

I want to merge this but CI on this PR is still remember, can you please check?

@reime005 reime005 requested a review from LeoNatan as a code owner November 10, 2018 21:17
@rotemmiz
Copy link
Member

A unit test still fails.
✕ installApp() with a custom app path should use custom app path

@rotemmiz
Copy link
Member

DTXLoggingInfra submodule has changed its reference, can you please revert that?

@rotemmiz
Copy link
Member

There is still one conflict :(

@reime005
Copy link
Contributor Author

There is still one conflict :(

should be fine now

@rotemmiz rotemmiz merged commit 9044414 into wix:master Nov 11, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Nov 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants