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

Various unit tests: Increase coverage #586

Merged
merged 7 commits into from Nov 5, 2020

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Nov 3, 2020

Update: I'm an idiot. The failure was due to the short duration, and the fact that there were only 120 frames available in the reader at the specified frame rate. (Though, interestingly, that doesn't throw an OutOfBoundsFrame it seems.)

As it says, this PR is a work-in-progress. Not because it's unfinished, but because a test is failing.

I decided to replace the first test (which had a catch in it that was screwing up the test coverage) with a simple no-op test: Create a mapping from 24fps to 24fps with no pulldown. Then read its Name() (adds an extra line of coverage), and finally read one frame from it. So, it just gets the same frame back. (When you use the right duration.)

Unfortunately, the test fails, and gets back frame 120 for original frame 125.
That seems bad.

Edit: Added a JSON test that just reads the Json() string from the mapper, then SetJson()s it back again (and spot-checks that things haven't changed.) That really raised the test coverage; I may do that for the other classes as well.

@ferdnyc ferdnyc requested a review from jonoomph November 3, 2020 13:15
tests/FrameMapper_Tests.cpp Outdated Show resolved Hide resolved
@ferdnyc ferdnyc changed the title WIP: FrameMapper_Tests: Use SUITE, create no-op test FrameMapper_Tests: Use SUITE, create no-op test Nov 3, 2020
@codecov
Copy link

codecov bot commented Nov 3, 2020

Codecov Report

Merging #586 into develop will increase coverage by 2.37%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #586      +/-   ##
===========================================
+ Coverage    49.90%   52.28%   +2.37%     
===========================================
  Files          132      131       -1     
  Lines        10686    10761      +75     
===========================================
+ Hits          5333     5626     +293     
+ Misses        5353     5135     -218     
Impacted Files Coverage Δ
tests/Color_Tests.cpp 100.00% <100.00%> (ø)
tests/FrameMapper_Tests.cpp 100.00% <100.00%> (+0.92%) ⬆️
tests/Point_Tests.cpp 100.00% <100.00%> (ø)
examples/qt-demo/main.cpp
src/FrameMapper.cpp 91.41% <0.00%> (+4.04%) ⬆️
src/Exceptions.h 29.85% <0.00%> (+4.47%) ⬆️
src/KeyFrame.cpp 85.94% <0.00%> (+5.22%) ⬆️
src/Json.cpp 100.00% <0.00%> (+9.09%) ⬆️
src/Coordinate.cpp 66.66% <0.00%> (+47.61%) ⬆️
... and 3 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 896b307...6b4ed3a. Read the comment docs.

@jonoomph
Copy link
Member

jonoomph commented Nov 3, 2020

LGTM!

@jonoomph
Copy link
Member

jonoomph commented Nov 3, 2020

Merge when ready!

- Add tests for second constructor, JSON in/out
@ferdnyc ferdnyc changed the title FrameMapper_Tests: Use SUITE, create no-op test Various unit tests: Increase coverage Nov 4, 2020
- Add blank constructor, JSON in/out tests

More Point tests
@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 4, 2020

I think I've run out of steam on this one for now. Just made a change to codecov.yml to hopefully get /examples out of the coverage report, and assuming that passes I'll merge. Coverage up from 49.90% (possibly higher, albeit artificially, if that config change takes), and newly 100% in three different files. I'd say that's a reasonable bump for one sortie.

@ferdnyc ferdnyc merged commit fb87cc7 into OpenShot:develop Nov 5, 2020
@ferdnyc ferdnyc deleted the framemapper-tests branch November 5, 2020 06:28
@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 5, 2020

(My attempt to exclude examples/ was... well, honestly, I'm not sure if it was successful.)

The two files that get built into targets by CMake still show up in the list, but Codecov doesn't seem to be counting any of their source lines for coverage purposes, which I guess is what really matters. Was hoping to get them off the list entirely, but meh...

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.

None yet

2 participants