Parallelize nightwatch #4481
Parallelize nightwatch #4481
Conversation
| initUserMock(level); | ||
| let tokenCounter = 0; | ||
|
|
||
| function getUserToken() { |
There was a problem hiding this comment.
I considered trying to use a selenium-based session id, but the session id is not sent in the request to the mock server, and using less magic around this felt more maintainable.
There was a problem hiding this comment.
That seems fine to me. (If I've learned anything about Selenium in the last few days, it's to not rely on its APIs...)
|
|
||
| function logIn(token, client, url, level) { | ||
| initUserMock(token, level); | ||
|
|
||
| client | ||
| .url(`${E2eHelpers.baseUrl}${url}`) | ||
| .waitForElementVisible('body', Timeouts.normal); |
There was a problem hiding this comment.
Unfortunately setting the users' session prior to waiting for the body to show on this page provided inconsistent results. I'd love if there were a way to set the session on the first page request so we didn't need to make two for every login. This may be possible by passing a session parameter in the URL or finding a way for the react application to read the users' session id from a method or the redux state instead of directly from local storage.
There was a problem hiding this comment.
Is this because localStorage isn't set until the body is already loaded? If that's the case, I'd guess that the URL paramater approach would be the way to go. It would just take a couple of changes in test-server.js, I think.
Something for a future PR, perhaps!
There was a problem hiding this comment.
I believe that's the case. I'll set up to do some more testing in the future if we find there's a good performance improvement to be had from this.
| }); | ||
| } | ||
|
|
||
| module.exports = mock; |
There was a problem hiding this comment.
This is somewhat out of convention - might consider changing this to export multiple methods or singularizing the name of the file.
There was a problem hiding this comment.
Seems fine for now. The helpers.js files aren't particularly idiomatic anyway...
| @@ -47,7 +45,12 @@ module.exports = { | |||
| log_path: './logs/selenium', | |||
| host: '127.0.0.1', | |||
| port: selenium_server_port, | |||
| } | |||
| }, | |||
| cli_args: { }, | |||
| cli_args: { }, | ||
| test_workers: { | ||
| enabled: true, | ||
| workers: 10, |
There was a problem hiding this comment.
10 seems excessive... I think "auto" is a better option here. Worth testing, maybe.
There was a problem hiding this comment.
I saw better responses discarding auto, which I believe defaults to the number of cpu's on the system. I'm guessing that the waits, pauses, and polling for elements to appear prevents the process from becoming cpu-bound.
| @@ -1,50 +1,46 @@ | |||
| if (!process.env.BUILDTYPE || process.env.BUILDTYPE === 'development') { | |||
There was a problem hiding this comment.
I'm guessing we are ditching these now that we check buildtype at the /script/run-nightwatch.js level. 👍
There was a problem hiding this comment.
In this case I removed the guard because these features are now included in staging and production, so the tests should be run against the build in all build types.
But yes, going forward #4497 provides a means of adequately testing features from a single build type, negating the need to guard test runs against development features at all.
| initUserMock(level); | ||
| let tokenCounter = 0; | ||
|
|
||
| function getUserToken() { |
There was a problem hiding this comment.
That seems fine to me. (If I've learned anything about Selenium in the last few days, it's to not rely on its APIs...)
|
|
||
| function logIn(token, client, url, level) { | ||
| initUserMock(token, level); | ||
|
|
||
| client | ||
| .url(`${E2eHelpers.baseUrl}${url}`) | ||
| .waitForElementVisible('body', Timeouts.normal); |
There was a problem hiding this comment.
Is this because localStorage isn't set until the body is already loaded? If that's the case, I'd guess that the URL paramater approach would be the way to go. It would just take a couple of changes in test-server.js, I think.
Something for a future PR, perhaps!
| }); | ||
| } | ||
|
|
||
| module.exports = mock; |
There was a problem hiding this comment.
Seems fine for now. The helpers.js files aren't particularly idiomatic anyway...
|
Test runtimes (observed):
|
…erans-affairs/vets-website into parallelize-nightwatch
…erans-affairs/vets-website into parallelize-nightwatch
Mock server implementation allows mocking requests based on user, which permits running multiple tests in parallel. Worker count set to n=10, may adjust while tuning performance for Jenkins environment.