Skip to content

Conversation

@elenatanasoiu
Copy link
Contributor

@elenatanasoiu elenatanasoiu commented Nov 1, 2022

This test has been quite flakey due to timeout errors:

1) "before each" hook: beforeEach for "should avoid creating a quick query":
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
      at listOnTimeout (node:internal/timers:559:17)
      at processTimers (node:internal/timers:502:7)

Since this is an async test, let's attempt to make all file operations use their asynchronous counterparts and see if this allows our test to finish on time.

Normally I'd consider increasing the timeout but the test itself doesn't load massive amounts of data so the culprit seems to be an unresolved promise.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@aeisenberg
Copy link
Contributor

The error happens in the beforeEach block, and so the current changes will not address that problem.

I'm pretty sure that the hanging happens in the calls to safeDel.

I've had problems with this test before. Maybe the best solution is to generate new values for qlFile and qlpackFile for each test run. And assume the temp files will be cleaned up after the test process finishes (true for linux/mac, not true for windows).

qlpackLockFile = `${ctx.storageUri?.fsPath}/quick-queries/codeql-pack.lock.yml`;
oldQlpackLockFile = `${ctx.storageUri?.fsPath}/quick-queries/qlpack.lock.yml`;
qlFile = `${ctx.storageUri?.fsPath}/quick-queries/quick-query.ql`;
qlFile = `${ctx.storageUri?.fsPath}/quick-queries/quick-query-${faker.name}.ql`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, this won't work since we expect a hard coded name for the quick query file.

I think we need to update the ctx.storageUri for each test and keep the rest of the path the same.

Copy link
Contributor Author

@elenatanasoiu elenatanasoiu Nov 28, 2022

Choose a reason for hiding this comment

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

Thanks for addressing this @aeisenberg. Closing this PR for now. If there are more flakey tests that pop-up I'll have another look. 👀

@elenatanasoiu elenatanasoiu deleted the elena/fix-flakey-test branch November 28, 2022 13:21
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 participants