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

Feat: unit tests for json_message and log_util #29

Merged
merged 7 commits into from
Oct 24, 2023

Conversation

mackenzie-grimes-noaa
Copy link
Contributor

@mackenzie-grimes-noaa mackenzie-grimes-noaa commented Oct 20, 2023

Linear Issue

N/A

Changes

  • Type hint cleanup for json_message.py
  • Add unit tests to cover json_message.py
  • Add unit tests to cover log_util.py
  • Remove unused code in path_builder.py that was hurting its coverage report

With these additions we should be above 90% coverage for this library.

@mackenzie-grimes-noaa mackenzie-grimes-noaa changed the title Feat/json message tests Feat: json message unit tests Oct 20, 2023
@mackenzie-grimes-noaa mackenzie-grimes-noaa marked this pull request as ready for review October 20, 2023 21:50
@mackenzie-grimes-noaa mackenzie-grimes-noaa changed the title Feat: json message unit tests Feat: unit tests for json_message and log_util Oct 20, 2023
Copy link
Contributor

@Geary-Layne Geary-Layne left a comment

Choose a reason for hiding this comment

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

Looks good
What made you decide to address these issue now?

@mackenzie-grimes-noaa
Copy link
Contributor Author

Looks good What made you decide to address these issue now?

I was already working in the unit tests for this repo, just having too much fun writing tests I suppose. It was maybe an extra hour of work, not worth 1 point

@mackenzie-grimes-noaa mackenzie-grimes-noaa merged commit b896095 into main Oct 24, 2023
@mackenzie-grimes-noaa mackenzie-grimes-noaa deleted the feat/json-message-tests branch October 24, 2023 22:39
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.

2 participants