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.

Sign up for GitHub

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

Update locking test #72

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update locking test #72

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41129b2
Update locking test
quarenw Jun 16, 2022
2b170a5
linting
quarenw Jun 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions test/PluginManagerSuite.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1114,21 +1114,15 @@ describe("PluginManager:", function() {
});

it("cannot install multiple package concurrently", async function() {
// I expect this to take some time...
const installation1 = manager.installFromNpm("moment");

// so I expect a concurrent installation to fail...
const pluginSourcePath = path.join(__dirname, "my-basic-plugin");
const installation1 = manager.installFromNpm("moment");
const installation2 = manager.installFromPath(pluginSourcePath);

try {
await installation2;
} catch (err) {
await installation1;
return;
await Promise.all([ installation1, installation2 ]);
Copy link
Owner

Choose a reason for hiding this comment

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

I see that with this change the next test fails. Maybe because one of these promise is not yet completed?
A possible solution can be to use Promise.allSettled instead of Promise.all?
So that we are sure that all installation are completed and there aren't any pending installation.
What do you think?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/allSettled

} catch (error) {
if (/failed to acquire lock/i.test(error as string)) return;
}

throw new Error("Expected to fail");
throw new Error("Expected to fail acquiring lock");
});

describe("given a lock", function() {
Expand Down