-
Notifications
You must be signed in to change notification settings - Fork 12k
test: use random e2e test ports #23541
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
Conversation
packages/schematics/angular/e2e/files/protractor.conf.js.template
Outdated
Show resolved
Hide resolved
8a53863
to
c74e33e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes other than that in https://github.com/angular/angular-cli/blob/c74e33eef1bbb4fc73e204520f57bba3574891dc/packages/angular_devkit/build_angular/src/builders/dev-server/specs/works_spec.ts should be reverted. For E2E the port should be set in
appTargets.e2e.options.webdriverUpdate = false; |
I don't think that catches all cases but looks like it fixes the ones actually using e2e so is probably enough 👍 |
appTargets.e2e.options.webdriverUpdate = false; | ||
// Use a random port in e2e. | ||
appTargets.e2e.options.port = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alan-agius4 do you think this would also be the ideal place to set appTargets.serve.options.port
? I just had a test running ng serve
fail in another PR due to 4200 being in use...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note there's various ng serve --port=0
throughout the tests already but a few are missing such as
rebuild-deps-type-check.ts
rebuild-types.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added another commit but feel free to only merge the first if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we should.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
* test: use random e2e test ports * test: use random ng serve ports (cherry picked from commit 35c4357)
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.