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

tests: Stop using nyc #4688

Merged
merged 1 commit into from Jan 29, 2021
Merged

tests: Stop using nyc #4688

merged 1 commit into from Jan 29, 2021

Conversation

rhansen
Copy link
Member

@rhansen rhansen commented Jan 29, 2021

There are some problems with nyc:

  • The coverage numbers aren't useful in our case because most of the code is executed outside the test process (the test code is mostly API client logic).
  • nyc messes with line numbers, which makes it much harder to debug problems.
  • We're seeing frequent SIGABRT crashes while nyc is printing the results table. I'm not sure if nyc is the cause of the crashes, or if it's making a race condition worse, or if the crashes have nothing to do with nyc, but we don't lose much by removing it so we might as well see if the crash frequency improves.

Addresses #4684.

There are some problems with nyc:
  * The coverage numbers aren't useful in our case because most of the
    code is executed outside the test process (the test code is mostly
    API client logic).
  * nyc messes with line numbers, which makes it much harder to debug
    problems.
  * We're seeing frequent SIGABRT crashes while nyc is printing the
    results table. I'm not sure if nyc is the cause of the crashes, or
    if it's making a race condition worse, or if the crashes have
    nothing to do with nyc, but we don't lose much by removing it so
    we might as well see if the crash frequency improves.
@JohnMcLear JohnMcLear merged commit 89bcfa2 into develop Jan 29, 2021
@JohnMcLear
Copy link
Member

@rhansen rhansen deleted the rhansen-nuke-nyc branch January 29, 2021 21:01
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