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

test: improve test cases #211

Closed
wants to merge 3 commits into from
Closed

Conversation

Daanvdplas
Copy link
Collaborator

@Daanvdplas Daanvdplas commented Aug 17, 2024

Refactors the the error handling in separate functions for easier testing between modules. The tests have also been refactored into separate tests for each error variant and more edge cases. More importantly, the tests don't test the Error from primitives but the raw bytes.

In #192, Error is refactored to the pop api crate, note that the pop primitives crate is removed from the extension crate in this PR since here it was forgotten.

In the second commit docs have been added to the v0 module as well.

Helping with #188

@Daanvdplas Daanvdplas changed the base branch from main to daan/api August 17, 2024 14:47
@Daanvdplas Daanvdplas changed the title tests: increase coverage test: increase coverage Aug 17, 2024
@codecov-commenter
Copy link

codecov-commenter commented Aug 17, 2024

Codecov Report

Attention: Patch coverage is 95.32164% with 8 lines in your changes missing coverage. Please review.

Project coverage is 32.76%. Comparing base (6adf1b1) to head (bdfdc96).

Files Patch % Lines
extension/src/lib.rs 22.22% 7 Missing ⚠️
extension/src/tests.rs 98.90% 0 Missing and 1 partial ⚠️
@@             Coverage Diff              @@
##           daan/api     #211      +/-   ##
============================================
- Coverage     34.08%   32.76%   -1.33%     
============================================
  Files            34       34              
  Lines          3013     2994      -19     
  Branches       3013     2994      -19     
============================================
- Hits           1027      981      -46     
- Misses         1954     1980      +26     
- Partials         32       33       +1     
Files Coverage Δ
extension/src/v0.rs 100.00% <100.00%> (ø)
extension/src/tests.rs 92.42% <98.90%> (+4.39%) ⬆️
extension/src/lib.rs 10.59% <22.22%> (+2.48%) ⬆️

... and 1 file with indirect coverage changes

@Daanvdplas Daanvdplas changed the title test: increase coverage test: improve test cases Aug 18, 2024
// - Represents the second level of nesting in `DOUBLE_NESTED_ERRORS`.
// - Byte 3:
// - Unused or represents further nested information.
pub(super) fn handle_unknown_error(encoded_error: &mut [u8; 4]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we assign this to variables? Similar to extract_env. Maybe

let (error_indx, error_type, ..double_nested_error) = encoded_error;

Or we can simply assign encoded_error[1..] to one variable because it is used in multiple places

@Daanvdplas
Copy link
Collaborator Author

No longer needed due to #218

@Daanvdplas Daanvdplas closed this Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants