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

fix(@angular/cli): fix webdriver deep import on yarn #4597

Closed
wants to merge 1 commit into from

Conversation

filipesilva
Copy link
Contributor

Fix #4596

@filipesilva
Copy link
Contributor Author

Doesn't seem like this fix is enough.

@Meligy
Copy link
Contributor

Meligy commented Feb 11, 2017

How about adding webdriver-manager directly to package.json instead of via protractor?

@cexbrayat
Copy link
Member

@filipesilva As you were wondering if this was enough, your PR fixes the issue for my project (cli beta.31 + yarn)

@victornoel
Copy link

@cexbrayat I guess it doesn't work with npm anymore and that's why it is not enough?

@filipesilva
Copy link
Contributor Author

Yeah it doesn't work npm with this fix. Not sure what the solution is for now, but the workaround is to update webdriver manually (($npm bin) webdriver-manager update) and to run the e2e command without webdriver (ng e2e --no-webdriver-update).

@Meligy
Copy link
Contributor

Meligy commented Feb 17, 2017

Would it make sense to run node_modules/.bin/webdriver-manager as a child process with the project as a working directory for the process?

@Meligy
Copy link
Contributor

Meligy commented Feb 17, 2017

That's essentially the CLI doing it the same way as the workaround.

@filipesilva
Copy link
Contributor Author

That's what we were trying to move away from, running npm scripts in separate processes.

@Meligy
Copy link
Contributor

Meligy commented Feb 26, 2017

I think there are potential 2 options:

1 - Add webdriver-manager to devDependencies so that it's hopefully always directly under node_modules. The con is having to sync its version to protractor manually

2 - Add a file check for one of the cases (say NPM, under protractor), and if it doesn't exist, revert back to looking in node_modules. The con is that it's just silly, and might not serve well in the future, but you can worry about this when there are many more cases.

I recommend just going with option 2, as this is a basic failing scenario at the moment.

@filipesilva
Copy link
Contributor Author

@Meligy I think something like 2 is a pretty good solution for now. Trying both and failing only if neither is there.

@dasch8 dasch8 mentioned this pull request Feb 26, 2017
filipesilva added a commit that referenced this pull request Feb 27, 2017
asnowwolf pushed a commit to asnowwolf/angular-cli that referenced this pull request Apr 12, 2017
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[beta.31] ng e2e does not work anymore
5 participants