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

Convert unit tests into subtest and to be table-driven #305

Merged
merged 6 commits into from
Feb 24, 2023
Merged

Convert unit tests into subtest and to be table-driven #305

merged 6 commits into from
Feb 24, 2023

Conversation

gpjservais
Copy link
Contributor

This is the final PR addressing refactoring tests from #284. This addresses the following tests:

  • TestMatch_IsEmpty
  • TestMatch_ValuesEmpty
  • TestNumber_EqualNaN
  • TestArray_IsEqual/types (split into subtests)
  • TestWebsocketMessage_CodeAndType
  • TestWebsocketMessage_NoContent

I converted all except TestArray_IsEqual to table driven style. I only split TestArray_IsEqual/types into subtests to address the multiple variable issue.

Also, as noted in the issue, I ended up merging TestNumber_EqualNaN into TestNumber_IsEqual (and also TestNumber_InDelta since half of the test related to InDelta/NotInDelta tests for NaN values). I ended up converting those tests to table-driven format as well.

This is ready for review.

@coveralls
Copy link
Collaborator

Coverage Status

Coverage: 95.129%. Remained the same when pulling 1465387 on gpjservais:284-refactor-other-unit-tests into 79ad562 on gavv:master.

@gavv gavv added the ready for review Pull request can be reviewed label Feb 23, 2023
@gavv
Copy link
Owner

gavv commented Feb 24, 2023

Thanks!

@gavv gavv merged commit b47b582 into gavv:master Feb 24, 2023
@gavv gavv removed the ready for review Pull request can be reviewed label Feb 24, 2023
@gpjservais gpjservais deleted the 284-refactor-other-unit-tests branch February 24, 2023 13:23
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