Skip to content

Fix lint race when not running tests in parallel#25235

Merged
weswigham merged 1 commit intomicrosoft:masterfrom
weswigham:fix-jakefile-race
Jun 26, 2018
Merged

Fix lint race when not running tests in parallel#25235
weswigham merged 1 commit intomicrosoft:masterfrom
weswigham:fix-jakefile-race

Conversation

@weswigham
Copy link
Copy Markdown
Member

Both runLinterAndComplete and finish were invoking complete (the first when linting was actually done, the second immediately), causing complete to be called for the lint's prerequisite build before the task was actually done (and more times and required), leading to lint running before is prerequisite tsbuild task was complete, resulting in an error if lint rules were not already built.

The jake API isn't exactly forgiving when it comes to ensuring your tasks only complete once when used in the way we've authored; as it turns out, jake supports async tasks returning promises (and if you do so, you no longer have to configure the async flag) - it would be much safer to be using them.

Copy link
Copy Markdown
Member

@RyanCavanaugh RyanCavanaugh left a comment

Choose a reason for hiding this comment

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

Explains a lot of weirdness. LGTM but why is the node-start project updated in this PR?

@weswigham
Copy link
Copy Markdown
Member Author

@RyanCavanaugh its reference updates anytime (user) tests are run (or submodules are updated); despite being ignored it gets added by git add . 🤷‍♂️ It doesn't really matter, since we always update the ref to latest before we check it, and it's a pain to purge from the commit once it's been added, since it's not a real file.

@weswigham weswigham merged commit 38dab74 into microsoft:master Jun 26, 2018
@weswigham weswigham deleted the fix-jakefile-race branch June 26, 2018 19:27
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants