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

Mocha test suite factory function should not be async #187718

Closed
7 tasks done
alexr00 opened this issue Jul 12, 2023 · 6 comments · Fixed by #189171
Closed
7 tasks done

Mocha test suite factory function should not be async #187718

alexr00 opened this issue Jul 12, 2023 · 6 comments · Fixed by #189171
Assignees
Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders
Milestone

Comments

@alexr00
Copy link
Member

alexr00 commented Jul 12, 2023

We discovered in #187482 (comment) that some of our suite factory methods are async and can impact other tests as a result. After discussion, we have decided that we should limit suite factory methods so they cannot be async.

The first step is to write an eslint rule to find async suite factory methods.

The second step is to fix all the places this occurs:

  • src\vs\editor\contrib\find\test\browser\findController.test.ts @alexdima
  • src\vs\workbench\contrib\search\test\common\extractRange.test.ts @Tyriar
  • src\vs\workbench\contrib\terminal\test\common\history.test.ts @Tyriar
  • src\vs\workbench\contrib\terminal\test\node\terminalProfiles.test.ts @Tyriar
  • src\vs\workbench\contrib\terminal\test\browser\xterm\decorationAddon.test.ts @meganrogge
  • src\vs\workbench\contrib\terminal\test\browser\xterm\shellIntegrationAddon.test.ts @Tyriar
  • src\vs\workbench\contrib\terminalContrib\quickFix\test\browser\quickFixAddon.test.ts @meganrogge
@alexr00 alexr00 added the debt Code quality issues label Jul 12, 2023
@alexr00 alexr00 added this to the July 2023 milestone Jul 12, 2023
@alexr00 alexr00 self-assigned this Jul 12, 2023
@alexr00 alexr00 modified the milestones: July 2023, August 2023 Jul 18, 2023
@alexr00 alexr00 changed the title Mocha test suite factory function should not do setup Mocha test suite factory function should not be asyn Jul 20, 2023
@alexr00 alexr00 changed the title Mocha test suite factory function should not be asyn Mocha test suite factory function should not be async Jul 20, 2023
alexr00 added a commit that referenced this issue Jul 20, 2023
@alexr00
Copy link
Member Author

alexr00 commented Jul 20, 2023

Once the PR for the eslint rule is in, you should just be able to see a warning by opening the file listed. To fix, make the second argument to the suite factory function sync (example: #188380).

If you need to run async code during your suite setup, consider using a suiteSetup.

@alexr00
Copy link
Member Author

alexr00 commented Jul 20, 2023

The PR for the lint rule causes hygiene to fail, so if you want to try the rule out to fix your change feel free to checkout #188378 and make your change on that branch.

@alexr00
Copy link
Member Author

alexr00 commented Jul 28, 2023

Now that debt week is beginning, please consider taking a look at the errors above so that we can merge this new eslint rule and prevent more unreliable tests to be written!

@Tyriar
Copy link
Member

Tyriar commented Jul 28, 2023

There were no awaits so I think we can just remove them all which I did in #189171, merging that will close this out.

Tyriar added a commit that referenced this issue Jul 28, 2023
@alexr00
Copy link
Member Author

alexr00 commented Jul 28, 2023

Great, thanks @Tyriar!

@Tyriar
Copy link
Member

Tyriar commented Jul 28, 2023

Oh opps, this will auto close. I see you will need to update the branch on #188378 and merge in before this is closed.

@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label Jul 28, 2023
alexr00 added a commit that referenced this issue Jul 28, 2023
@VSCodeTriageBot VSCodeTriageBot added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Aug 3, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants