Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Guard test against potential races #567

Merged
merged 2 commits into from
Sep 12, 2019
Merged

Conversation

xtuc
Copy link
Member

@xtuc xtuc commented Sep 12, 2019

When you run wrangler build it will try to install the npm
dependencies of the user's project and, in development mode, the
one from wranglerjs. This can lead to races if two build were launch
at the same time. To avoid that we use a file lock and make wrangler
wait if it's already locked. However, it turns out that wrangler will
not wait and run npm install regardless.

This change avoids running multiple wrangler build at the same time
during our unit tests.

xtuc and others added 2 commits September 12, 2019 17:32
When you run `wrangler build` it will try to install the `npm`
dependencies of the user's project and, in development mode, the
one from `wranglerjs`. This can lead to races if two build were launch
at the same time. To avoid that we use a file lock and make wrangler
wait if it's already locked. However, it turns out that wrangler will
not wait and run `npm install` regardless.

This change avoids running multiple `wrangler build` at the same time
during our unit tests.
Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@ashleygwilliams ashleygwilliams left a comment

Choose a reason for hiding this comment

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

long term, we likely want a diff solution, but this seems OK for now

@EverlastingBugstopper EverlastingBugstopper merged commit 41c2154 into master Sep 12, 2019
@delete-merged-branch delete-merged-branch bot deleted the hotfix-guard-test branch September 12, 2019 19:35
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.

3 participants