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

Adding the possibility to specify a test runner path besides from local ./node_modules #753

Closed
wants to merge 10 commits into from

Conversation

moQuez
Copy link

@moQuez moQuez commented May 24, 2018

This PR is for issue #752, I hope you guys find it useful since it is the only way I can make it work on my current project 👐

@moQuez moQuez requested a review from rotemmiz as a code owner May 24, 2018 20:21
@moQuez
Copy link
Author

moQuez commented May 25, 2018

fixed a conflict with my "runnerPath" and @simonbuchan "binPath".

Copy link
Contributor

@simonbuchan simonbuchan left a comment

Choose a reason for hiding this comment

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

Not a fan of all the format diffs, though it does look like there is some mixed indentation already.

Instead of adding another property, this could instead check if the runner has a path separator (/ on posix, either / or \ on windows), and simply not path join it in that case: e.g. ./my-script is in the working dir. For windows support, you still should normalise the path to use \ - it's only required for commands run by the windows shell, but that's what's happening here.

@moQuez
Copy link
Author

moQuez commented May 26, 2018

thanks @simonbuchan, I actually removed a binPath left behind... As I submitted the PR before your's got merged I didn't resolve the conflicts properly because I was working on something else... Now everything should be fine 🙌

@moQuez
Copy link
Author

moQuez commented May 30, 2018

@simonbuchan @rotemmiz anything else that I need for this to get merged?

@moQuez moQuez closed this Jun 24, 2018
@rotemmiz
Copy link
Member

rotemmiz commented Jul 9, 2018

I'm trying to think this through, but I fail to get convinced this is valid solution. Seems like your project setup might get encountered in many issues of this type on other tools, not just Detox. I have a feeling that in your usecase it might be smarter to just add the .bin of other projects in the monorepo to your project path.

@wix wix locked and limited conversation to collaborators Jul 23, 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