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

Extend unit tests for BMP & Datasets #2060

Merged
merged 14 commits into from
Feb 4, 2022
Merged

Extend unit tests for BMP & Datasets #2060

merged 14 commits into from
Feb 4, 2022

Conversation

piponazo
Copy link
Collaborator

@piponazo piponazo commented Jan 27, 2022

In this PR I am continuing with the addition of unit tests for some parts of the code that I am analysing. I try to focus on some areas of the code which were not covered by either unit tests or the python tests.

NOTE: I am also opening the PR as DRAFT so that I can check what is going wrong with the codecov reports. I'll do some modifications there aiming to get more stable results.
UPDATE: I found out that the difference between codecov (61.49%) and the local coverage reports (75.1%) is due to the different treatment of partial coverage. While locally a partial coverage is treated as a HIT, in codecov is treated as a MISS: https://docs.codecov.com/docs/frequently-asked-questions#how-is-coverage-calculated

@codecov
Copy link

codecov bot commented Jan 27, 2022

Codecov Report

Merging #2060 (a425018) into main (8505f4d) will increase coverage by 0.34%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2060      +/-   ##
==========================================
+ Coverage   61.48%   61.83%   +0.34%     
==========================================
  Files          96       96              
  Lines       19207    19100     -107     
  Branches     9843     9817      -26     
==========================================
  Hits        11810    11810              
+ Misses       5080     4995      -85     
+ Partials     2317     2295      -22     
Impacted Files Coverage Δ
include/exiv2/datasets.hpp 100.00% <ø> (ø)
include/exiv2/image.hpp 100.00% <ø> (ø)
include/exiv2/iptc.hpp 100.00% <ø> (ø)
src/error.cpp 74.41% <ø> (ø)
src/bmpimage.cpp 90.00% <66.66%> (+68.37%) ⬆️
src/datasets.cpp 95.45% <91.80%> (+48.67%) ⬆️
src/image.cpp 64.28% <100.00%> (+0.29%) ⬆️
src/safe_op.hpp 69.23% <0.00%> (-27.65%) ⬇️
include/exiv2/slice.hpp 69.64% <0.00%> (-21.89%) ⬇️
include/exiv2/error.hpp 60.71% <0.00%> (-4.81%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8505f4d...a425018. Read the comment docs.

@piponazo piponazo requested review from hassec, clanmills, kevinbackhouse and kmilos and removed request for hassec January 27, 2022 15:39
@piponazo piponazo marked this pull request as ready for review January 27, 2022 15:40
@piponazo piponazo self-assigned this Jan 27, 2022
hassec
hassec previously approved these changes Feb 4, 2022
Copy link
Member

@hassec hassec left a comment

Choose a reason for hiding this comment

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

Had a brief look and I think this all looks good 👍

really happy to see more testing! 😊

Copy link
Member

@hassec hassec left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@piponazo piponazo merged commit ed09d9f into main Feb 4, 2022
@piponazo piponazo deleted the main_bmpTests branch February 4, 2022 17:44
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