Skip to content

Fix unittest discovery when using VS Code Insiders by using Inversify's skipBaseClassChecks option.#15049

Merged
kimadeline merged 3 commits intomicrosoft:mainfrom
kimadeline:14962-unittest-macos
Jan 4, 2021
Merged

Fix unittest discovery when using VS Code Insiders by using Inversify's skipBaseClassChecks option.#15049
kimadeline merged 3 commits intomicrosoft:mainfrom
kimadeline:14962-unittest-macos

Conversation

@kimadeline
Copy link
Copy Markdown

@kimadeline kimadeline commented Dec 23, 2020 •
edited
Loading

For #14962

What skipBaseClassCheck does: https://github.com/inversify/InversifyJS/blob/master/wiki/container_api.md#skipbaseclasschecks

You can use this to skip checking base classes for the @Injectable property, which is especially useful if any of your @Injectable classes extend classes that you don't control (third party classes). By default, this value is false.

UnitTestSocketServer (which is causing the issue) extends from EventEmitter, a Node class we don't control.

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
  • Title summarizes what is changing.
  • Has a news entry file (remember to thank yourself!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • Unit tests & system/integration tests are added/updated.
  • Test plan is updated as appropriate.
  • package-lock.json has been regenerated by running npm install (if dependencies have changed).
  • The wiki is updated with any design decisions/details.

@kimadeline kimadeline self-assigned this Dec 23, 2020
@kimadeline kimadeline marked this pull request as ready for review December 23, 2020 19:06
karrtikr
karrtikr approved these changes Dec 23, 2020
View reviewed changes
Copy link
Copy Markdown

@karrtikr karrtikr left a comment

Choose a reason for hiding this comment

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

Interesting, does this actually fix the unit test issue for you?

@kimadeline
Copy link
Copy Markdown
Author

kimadeline commented Dec 23, 2020

Yep, and I checked, unit tests are still discovered when using VS Code stable, it doesn't seem to have broken anything as far as I can see.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 23, 2020

Codecov Report

Merging #15049 (2fe60cd) into main (91ff97f) will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##            main   #15049   +/-   ##
======================================
  Coverage     65%      65%           
======================================
  Files        558      558           
  Lines      26231    26231           
  Branches    3728     3728           
======================================
  Hits       17155    17155           
  Misses      8381     8381           
  Partials     695      695

@kimadeline kimadeline merged commit 4d1037b into microsoft:main Jan 4, 2021
@kimadeline kimadeline deleted the 14962-unittest-macos branch January 4, 2021 19:24
@karrtikr karrtikr mentioned this pull request Feb 9, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@brettcannon brettcannon brettcannon approved these changes

@karrtikr karrtikr karrtikr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kimadeline kimadeline

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kimadeline @codecov-io @brettcannon @karrtikr