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

Make console-output functions redirectable; add operator<< for Fraction, Point, Coordinate; new KeyFrame::PrintPoints & PrintValues with unit tests #661

Merged
merged 15 commits into from
Sep 27, 2021

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Apr 20, 2021

Redirectable output functions

To enable unit-testing of functions like ReaderBase::DisplayInfo() and Keyframe::PrintPoints() (which normally dump information to std::cout), each such function has been updated to have a signature like this:

/// Display effect information in the standard output stream (stdout)
void DisplayInfo(std::ostream* out=&std::cout);

The function now takes a pointer to the output stream to use, with std::cout being only the overridable default for that stream. In the output function, *out is streamed to instead of the hardcoded std::cout:

void WriterBase::DisplayInfo(std::ostream* out) {
	*out << std::fixed << std::setprecision(2) << std::boolalpha;
	*out << "----------------------------" << std::endl;
	*out << "----- File Information -----" << std::endl;
	*out << "----------------------------" << std::endl;
	*out << "--> Has Video: " << info.has_video << std::endl;
	*out << "--> Has Audio: " << info.has_audio << std::endl;

This makes it possible to unit test the functions by passing an alternate stream, then examining the captured output:

std::string expected(R"(----------------------------
----- File Information -----
----------------------------
--> Has Video: true
--> Has Audio: true
--> Has Single Image: false
--> Duration: 51.95 Seconds
--> File Size: 7.26 MB
----------------------------
----- Video Attributes -----
----------------------------
--> Width: 1280
--> Height: 720)");

// Store the DisplayInfo() text in 'output'
std::stringstream output;
r.DisplayInfo(&output);

// Compare a [0, expected.size()) substring of output to expected
auto compare_value = output.str().compare(0, expected.size(), expected);
CHECK(compare_value == 0);

Unit tests for ReaderBase::DisplayInfo() (via FFmpegReader), WriterBase::DisplayInfo() (via FFmpegWriter), and FrameMapper::PrintMapping() are included in the PR.

Output-stream compatible formatting for Coordinate, Point, Fraction

Also, the operator<< overloads taking openshot::Coordinate, openshot::Point, and openshot::Fraction which I'd written for another PR that ended up on hold are included here, along with their unit tests.

Updated Keyframe::PrintPoints() and Keyframe::PrintValues()

The new operator<<(openshot::Fraction) is used as part of a new KeyFrame::PrintValues() implementation; KeyFrame::PrintPoints() is also updated. Both have new unit tests. Sample output:

>>> import openshot
>>> k = openshot.Keyframe([
...     openshot.Point(1, 10), openshot.Point(5, 20), openshot.Point(6, -4), openshot.Point(10, 1)
... ])
>>> k.PrintPoints()
     1       10.0000
     5       20.0000
     6       -4.0000
    10        1.0000
>>> k.PrintValues()
│Frame# (X) │     Y Value │ Delta Y │ Increasing? │ Repeat Fraction    │
├───────────┼─────────────┼─────────┼─────────────┼────────────────────┤
│       1 *10.0000+10trueFraction(1, 1)     │
│       211.0702+1trueFraction(1, 1)     │
│       315.0000+4trueFraction(1, 1)     │
│       418.9298+4trueFraction(1, 1)     │
│       5 *20.0000+1falseFraction(1, 1)     │
│       6 *-4.0000-24trueFraction(1, 1)     │
│       7-3.4649+1trueFraction(1, 1)     │
│       8-1.5000+1trueFraction(1, 1)     │
│       90.4649+2trueFraction(1, 1)     │
│      10 *1.0000+1trueFraction(1, 1)     │
 * = Keyframe point (non-interpolated)

(Ick! Way to screw up the formatting, Github! Well, it looks right on my screen:)
image

@ferdnyc ferdnyc added the tests Changes related to the unit tests and/or code coverage label Apr 20, 2021
@ferdnyc
Copy link
Contributor Author

ferdnyc commented Apr 20, 2021

A unit test for ReaderBase::DisplayInfo() is included in the PR, the others are left as an exercise for the reader.

LOL. Which, of course, leads to CodeCov spanking me for not having sufficient coverage of the lines changed in the PR. Typical.

@codecov
Copy link

codecov bot commented Apr 20, 2021

Codecov Report

Merging #661 (e5d70a8) into develop (7e419b9) will increase coverage by 1.06%.
The diff coverage is 95.42%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #661      +/-   ##
===========================================
+ Coverage    46.24%   47.31%   +1.06%     
===========================================
  Files          182      185       +3     
  Lines        16701    16839     +138     
===========================================
+ Hits          7724     7968     +244     
+ Misses        8977     8871     -106     
Impacted Files Coverage Δ
src/AudioReaderSource.cpp 0.86% <0.00%> (-0.01%) ⬇️
src/Color.cpp 88.88% <ø> (ø)
src/EffectBase.cpp 20.58% <0.00%> (ø)
src/EffectBase.h 50.00% <ø> (ø)
src/FrameMapper.h 100.00% <ø> (ø)
src/KeyFrame.h 100.00% <ø> (ø)
src/ReaderBase.h 100.00% <ø> (ø)
src/WriterBase.h 100.00% <ø> (ø)
src/ClipBase.h 75.00% <100.00%> (-5.77%) ⬇️
src/Coordinate.cpp 100.00% <100.00%> (ø)
... and 22 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 7e419b9...e5d70a8. Read the comment docs.

@github-actions github-actions bot added the conflicts A PR with unresolved merge conflicts label Jun 1, 2021
@github-actions
Copy link

github-actions bot commented Jun 1, 2021

Merge conflicts have been detected on this PR, please resolve.

@ferdnyc ferdnyc removed the conflicts A PR with unresolved merge conflicts label Jun 4, 2021
@github-actions
Copy link

github-actions bot commented Jun 5, 2021

Merge conflicts have been detected on this PR, please resolve.

@github-actions github-actions bot added the conflicts A PR with unresolved merge conflicts label Jun 5, 2021
@ferdnyc ferdnyc removed the conflicts A PR with unresolved merge conflicts label Jun 5, 2021
@ferdnyc ferdnyc changed the title Make console-output functions redirectable, and therefore testable Make console-output functions redirectable; add operator<< for Fraction, Point, Coordinate; new KeyFrame::PrintPoints & PrintValues with unit tests Jun 11, 2021
- The function now takes a pointer to the output stream it will
  write to. The _default_ for that argument is a pointer to std::cout.
- Any unit tests which wish to test the functionality can capture
  the output by passing an alternate buffer:
    std::stringstream output;
    reader.DisplayInfo(&output);
    CHECK(output.str() == "Expected output");
@ferdnyc
Copy link
Contributor Author

ferdnyc commented Sep 27, 2021

Well, having heard nothing on this after many months, I'm going to merge it. 😞

@ferdnyc ferdnyc merged commit b8b5505 into OpenShot:develop Sep 27, 2021
@ferdnyc ferdnyc deleted the redirect-print-fxns branch September 27, 2021 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Changes related to the unit tests and/or code coverage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant