Skip to content

Conversation

@erlend-aasland
Copy link
Contributor

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Jul 1, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.84%. Comparing base (503918c) to head (6b1d8ae).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #474      +/-   ##
==========================================
+ Coverage   67.68%   67.84%   +0.16%     
==========================================
  Files          26       26              
  Lines        3104     3104              
  Branches      521      521              
==========================================
+ Hits         2101     2106       +5     
+ Misses        862      859       -3     
+ Partials      141      139       -2     

see 2 files with indirect coverage changes

@erlend-aasland
Copy link
Contributor Author

Moreover, don't shadow tempfile; use instead the parameter name of the API: dest.

@erlend-aasland erlend-aasland changed the title Test export_eds() to stdout Increase export_eds() test coverage Jul 1, 2024
@erlend-aasland
Copy link
Contributor Author

I expanded the scope of the PR with 083c24c; I've split up the tests in multiple test functions, for easier debugging in case one of them fails. I also used subTest to for a more user-friendly response in case a test fails. Added tests for some more paths through export_od().

@acolomb
Copy link
Member

acolomb commented Jul 2, 2024

Waiting for #476, as that will change the raised exception type.

@erlend-aasland

This comment was marked as resolved.

@erlend-aasland erlend-aasland requested a review from acolomb July 2, 2024 10:59
@acolomb acolomb merged commit e3af0eb into canopen-python:master Jul 2, 2024
@erlend-aasland erlend-aasland deleted the test/export-od-write-to-stdout branch July 2, 2024 11:30
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Jul 2, 2024

Thanks for the review! Thanks to @sveinse for the yak-shave in 5db5913.

@erlend-aasland
Copy link
Contributor Author

Ouch, I seem to have introduced a temporary file mess when running the tests. Sorry 'bout that; I'll create a PR to ensure tests are properly cleaned up.

erlend-aasland added a commit to erlend-aasland/canopen that referenced this pull request Jul 2, 2024
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