Skip to content

🙈 Fix coverage ignore statement for racey errors#448

Merged
wwilsman merged 2 commits intomasterfrom
ww/fix-coverage-ignore-race
Jul 27, 2021
Merged

🙈 Fix coverage ignore statement for racey errors#448
wwilsman merged 2 commits intomasterfrom
ww/fix-coverage-ignore-race

Conversation

@wwilsman
Copy link
Copy Markdown
Contributor

What is this?

Unfortunately, while #408 appeared to address a race between browser and page error handling, there existed other codepaths that could also race in the same way for the same reason.

For this codepath, the code was formatted in a way where we were only ignoring line coverage. The function though, is still created and in certain circumstances may not be called (which was the original goal of this ignore statement).

This PR updates the formatting and ignore statement so the entire handler is ignored. I found this "missing" coverage by studying the coverage output and figuring out which functions were being hit the least. The crash handler was only hit once, but we explicitly test for it. However, I noticed the ignore statement there and also in the close method in which it was ignoring. This tipped me off that the ignore statement in the close method was in the wrong place, and subsequently the error handler in the crash handler was unnecessary.

I don't know for sure if this will fix our flakey coverage, but I have a strong hunch it might.

@wwilsman wwilsman added the 🧹 maintenance General maintenance label Jul 27, 2021
@wwilsman wwilsman requested a review from Robdel12 July 27, 2021 01:29
@wwilsman
Copy link
Copy Markdown
Contributor Author

wwilsman commented Jul 27, 2021

🤔 ...ignore statement not working?

@wwilsman wwilsman enabled auto-merge (squash) July 27, 2021 02:02
Copy link
Copy Markdown
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

🏁 YES PLEASE 👏🏼

@wwilsman wwilsman merged commit bc51025 into master Jul 27, 2021
@wwilsman wwilsman deleted the ww/fix-coverage-ignore-race branch July 27, 2021 12:26
samarsault pushed a commit that referenced this pull request Mar 3, 2023
Bumps [tsd](https://github.com/SamVerschueren/tsd) from 0.19.0 to 0.19.1.
- [Release notes](https://github.com/SamVerschueren/tsd/releases)
- [Commits](tsdjs/tsd@v0.19.0...v0.19.1)

---
updated-dependencies:
- dependency-name: tsd
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🧹 maintenance General maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants