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

Update typings for mocha #752

Merged
merged 2 commits into from Feb 12, 2021

Conversation

aeisenberg
Copy link
Contributor

@aeisenberg aeisenberg commented Feb 11, 2021

Internal change. Also, updates the lock file format for compatibility with npm v7.

Also, see the second commit:

Fix globalSetup/teardown for mocha

Updating the typings for mocha uncovered an error in how we were
registering global setups and teardowns.

When calling mocha.globalSetup or mocha.globalTeardown, any
previously registered globals are overwritten. The workaround
is to attach globals directly to the internal options object.

This is a requirement because we are now registering globals in
multiple files.

Unfortunately, the typings for mocha do not permit this and I may need
to fix them again.

Checklist

  • [n/a] CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • [n/a] Issues have been created for any UI or other user-facing changes made by this pull request.
  • [n/a] @github/docs-content-dsp has been cc'd in all issues for UI or other user-facing changes made by this pull request.

@aeisenberg aeisenberg requested review from alexet and adityasharad Feb 11, 2021
extensions/ql-vscode/package-lock.json Show resolved Hide resolved
@aeisenberg aeisenberg force-pushed the aeisenberg/update-mocha-typings branch from 2512d38 to 065bcf1 Compare Feb 11, 2021
@aeisenberg
Copy link
Contributor Author

@aeisenberg aeisenberg commented Feb 11, 2021

Concerning that the integration tests are failing here. I'll have to explore why.

This is includes an update of the lock file to the v2 format. It's a big
change, but not much is happening here. I thought it best to keep it
separate.
@aeisenberg
Copy link
Contributor Author

@aeisenberg aeisenberg commented Feb 12, 2021

Another review, please. The failing tests uncovered a problem with the way we are registering globals. It hasn't affected anything yet, but it's likely to do so in the future.

@aeisenberg
Copy link
Contributor Author

@aeisenberg aeisenberg commented Feb 12, 2021

Thanks. But, something's still weird and tests are stil failing. I'll have to inspect more.

@adityasharad
Copy link
Contributor

@adityasharad adityasharad commented Feb 12, 2021

Do the downloaded CLI and the test DB need to be registered as globals?

Updating the typings for mocha uncovered an error in how we were
registering global setups and teardowns.

When calling `mocha.globalSetup` or `mocha.globalTeardown`, any
previously registered globals are overwritten. The workaround
is to attach globals directly to the internal options object.

This is a requirement because we are now registering globals in
multiple files.

Unfortunately, the typings for mocha do not permit this and I may need
to fix them again.
@aeisenberg aeisenberg force-pushed the aeisenberg/update-mocha-typings branch from 4f1b11c to bbe3156 Compare Feb 12, 2021
@aeisenberg
Copy link
Contributor Author

@aeisenberg aeisenberg commented Feb 12, 2021

It was just a bad merge. I missed a few bits. Should be working now.

@adityasharad adityasharad merged commit 6304fe0 into github:main Feb 12, 2021
12 checks passed
aofaof0907 added a commit to aofaof0907/vscode-codeql that referenced this issue Jul 27, 2021
* Update typings for mocha

This is includes an update of the lock file to the v2 format. It's a big
change, but not much is happening here. I thought it best to keep it
separate.

* Fix globalSetup/teardown for mocha

Updating the typings for mocha uncovered an error in how we were
registering global setups and teardowns.

When calling `mocha.globalSetup` or `mocha.globalTeardown`, any
previously registered globals are overwritten. The workaround
is to attach globals directly to the internal options object.

This is a requirement because we are now registering globals in
multiple files.

Unfortunately, the typings for mocha do not permit this and I may need
to fix them again.
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.

None yet

2 participants