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 sync tests getting skipped in parallel mode. #3791

Merged
merged 5 commits into from
Jul 18, 2023

Conversation

garg3133
Copy link
Member

@garg3133 garg3133 commented Jul 4, 2023

Fixes: #3786.

The issue here was that custom commands many times return an instance of NightwatchAPI instead of the actual command result. Due to this, when destructuring status from result, status was getting assigned the definition of .status() (a function) command instead of the status of the command run (a number).

Now, while being in parallel mode, reports from all test runs are sent to a common port (see here), but the postMessage method does not work with functions, and so it was failing for tests containing custom commands.

Due to this, while the test with custom commands was running fine, it was not getting reported on the terminal as well as in the HTML report, hence it seemed like it was getting skipped.

@github-actions
Copy link

github-actions bot commented Jul 4, 2023

Status

  • ❌ No modified files found in the types directory.
    Please make sure to include types for any changes you have made. Thank you!.

@garg3133 garg3133 changed the title Fix sync tests being skipped in parallel mode. Fix sync tests getting skipped in parallel mode. Jul 4, 2023
@swrdfish
Copy link
Member

@garg3133 were you able to add a test for this?

@garg3133
Copy link
Member Author

@swrdfish I wasn't able to get it working, and after that my focus shifted to the namespace thing. But the gist of it is:

During parallel testing, Nightwatch creates separate workers for each test suite, but while doing so, it does not pass the custom settings defined in our tests and due to this, the settings defined in tests are not obeyed. And when I tried to modify the code so that it passes the settings to each worker, I'm getting the same error that this PR is solving, but at a different place (because settings may contain a function, Nightwatch is unable to pass the settings object to the workers).

Copy link
Member

@swrdfish swrdfish left a comment

Choose a reason for hiding this comment

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

After a debug session with @garg3133 , it looks like this fix needs more work.

@garg3133
Copy link
Member Author

This PR is ready now.

@garg3133
Copy link
Member Author

garg3133 commented Jul 13, 2023

@AutomatedTester This also needs to go in our next release.

/cc: @gravityvi since this PR makes some changes around the work you did on worker threads. (Those changes are required to be able to use parallel testing in our unit tests, and have no connection with the actual issue.)

lib/runner/concurrency/worker-task.js Outdated Show resolved Hide resolved
lib/reporter/index.js Outdated Show resolved Hide resolved
@garg3133
Copy link
Member Author

garg3133 commented Jul 18, 2023

@gravityvi @swrdfish Can you please review this PR once more? I've made some changes as suggested by Ravi and this PR looks good to me now.

(The let -> const changes in testsuite/index.js are done by Husky automatically.)

lib/testsuite/index.js Show resolved Hide resolved
lib/runner/concurrency/worker-task.js Show resolved Hide resolved
@AutomatedTester AutomatedTester merged commit 0088762 into nightwatchjs:main Jul 18, 2023
17 checks passed
yashPratp983 pushed a commit to yashPratp983/nightwatch that referenced this pull request Jul 25, 2023
yashPratp983 pushed a commit to yashPratp983/nightwatch that referenced this pull request Jul 25, 2023
yashPratp983 pushed a commit to yashPratp983/nightwatch that referenced this pull request Jul 25, 2023
yashPratp983 pushed a commit to yashPratp983/nightwatch that referenced this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3.0.1 silently skipping tests with async commands not used in async tests
4 participants