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

Run coverage on whole workspace #71

Merged
merged 4 commits into from
Oct 9, 2023
Merged

Run coverage on whole workspace #71

merged 4 commits into from
Oct 9, 2023

Conversation

poszu
Copy link
Collaborator

@poszu poszu commented May 18, 2023

No description provided.

github-actions[bot]

This comment was marked as outdated.

@poszu poszu force-pushed the improve-coverage-reporting branch from dee12e0 to 344f34c Compare May 18, 2023 11:12
github-actions[bot]
github-actions bot previously approved these changes May 18, 2023
@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #71 (a0bb845) into main (d15a6d2) will decrease coverage by 2.40%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #71      +/-   ##
==========================================
- Coverage   97.30%   94.90%   -2.40%     
==========================================
  Files          13       20       +7     
  Lines        1632     2691    +1059     
==========================================
+ Hits         1588     2554     +966     
- Misses         44      137      +93     
Files Coverage Δ
ffi/src/log.rs 85.71% <100.00%> (ø)

... and 7 files with indirect coverage changes

@github-actions github-actions bot dismissed their stale review May 18, 2023 11:44

confidence: 0.46 | [dashboard]

@poszu poszu force-pushed the improve-coverage-reporting branch from 612c933 to e4a2ccb Compare October 3, 2023 14:37
@poszu poszu requested review from github-actions[bot] and removed request for github-actions[bot] October 3, 2023 14:57
@poszu poszu marked this pull request as ready for review October 3, 2023 15:04
@@ -77,3 +77,60 @@ pub extern "C" fn set_logging_callback(
}
}
}

#[cfg(test)]
Copy link
Contributor

Choose a reason for hiding this comment

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

i recently learned that there is an easy way to split out tests into separate file, e.g more like in golang. do you like that or not so much?

#[cfg(test)]
#[path = "log_test.rs"]
pub mod test;

and then in log_test.rs

 #[test]
fn test_c_logger() {
...
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, it is possible, but by convention in Rust, the unit tests go in the same file as the implementation (https://doc.rust-lang.org/book/ch11-03-test-organization.html). I think it makes sense to keep them close, especially if the tests are simple, there aren't many of them. If there are many/they are complex, it's an indication that maybe they should be integration tests (these land in separate tests directory.

@poszu poszu merged commit 7e25b83 into main Oct 9, 2023
17 of 18 checks passed
@poszu poszu deleted the improve-coverage-reporting branch October 9, 2023 11: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.

2 participants