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

CMake: Add separate unit test targets #660

Merged
merged 3 commits into from Apr 19, 2021

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Apr 19, 2021

Each unit test file ClassName.cpp, which results in the creation of a test target openshot-ClassName-test, will now also be labeled with the CTest label ClassName, and a target ClassName_coverage will be generated that runs only the tests in that file.

The names

Just like the main coverage target, the target name ClassName_coverage is just a generic name. Running the target will collect coverage data if ENABLE_COVERAGE is set, but will run the tests without coverage if it is left disabled. It does not imply collection of coverage data.

(That all started because I couldn't redefine the test target to also collect coverage, so I had to use a different name. And since it's easier to always use the same target name, I made coverage work to run tests with or without ENABLE_COVERAGE. I could have called the individual targets ClassName_test, but I decided to be consistent with coverage.)

Y THO?

The separate targets are especially useful when developing tests for a class, as the tests in the class being worked on can be re-run without having to wait a minute or more while the other tests run.

Example workflow

# Configure project
cmake -B build -S .
# Work on tests for MyClass
vim tests/MyClass.cpp
# Build and run only the tests for MyClass
cmake --build build --target MyClass_coverage

(The first time through, libopenshot itself will still have to be built before the tests can run. But in subsequent executions of cmake --build build --target MyClass_coverage, the build will be quick because the only file that will be recompiled is tests/MyClass.cpp. When running the tests, the very lengthy runtimes of certain tests in FrameMapper, FFmpegWriter, and CVTracker are avoided.)

Each unit test file ClassName.cpp, which results in the creation
of a test target openshot-ClassName-test, will now also be labeled
with the CTest label ClassName, and a target ClassName_coverage
will be generated that runs only the tests in that file.

This is especially useful when developing tests for a class, as
the tests in the class being worked on can be re-run without
having to wait a minute or more while the other tests run.
@ferdnyc
Copy link
Contributor Author

ferdnyc commented Apr 19, 2021

When running the tests, the very lengthy runtimes of certain tests in FrameMapper, FFmpegWriter, and CVTracker are avoided.

In fact, the labeling has an unexpected side benefit: This summary, which CTest displays at the end of the run if the full suite of tests are enabled:

100% tests passed, 0 tests failed out of 135

Label Time Summary:
CVStabilizer     =   3.56 sec*proc (2 tests)
CVTracker        =  62.05 sec*proc (2 tests)
CacheDisk        =   6.82 sec*proc (6 tests)
CacheMemory      =  10.67 sec*proc (11 tests)
Clip             =   7.18 sec*proc (6 tests)
Color            =   8.81 sec*proc (9 tests)
Coordinate       =   5.10 sec*proc (5 tests)
DummyReader      =   7.08 sec*proc (7 tests)
FFmpegReader     =  14.32 sec*proc (8 tests)
FFmpegWriter     =  10.08 sec*proc (2 tests)
Fraction         =   4.33 sec*proc (5 tests)
Frame            =   5.55 sec*proc (5 tests)
FrameMapper      =  38.04 sec*proc (13 tests)
ImageWriter      =   5.00 sec*proc (1 test)
KeyFrame         =  23.89 sec*proc (24 tests)
Point            =  13.41 sec*proc (11 tests)
QtImageReader    =   3.54 sec*proc (3 tests)
ReaderBase       =   1.14 sec*proc (1 test)
Settings         =   1.96 sec*proc (2 tests)
Timeline         =  14.52 sec*proc (12 tests)

Total Test time (real) =  63.19 sec

So we can now see at a glance that CVTracker is by far the most time-consuming test (as one would expect, really, given the complexity of its function), but FrameMapper is shockingly the second-place occupant on that list, which feels worth investigating.

The long runtime may simply be a result of test thoroughness and/or complexity. (That's certainly why KeyFrame takes so long. It runs 24 separate test cases, almost twice as many as FrameMapper.) But long runtimes may also may be a sign of inefficiencies — either in the tests, or the class itself.

@codecov
Copy link

codecov bot commented Apr 19, 2021

Codecov Report

Merging #660 (317c0be) into develop (4e4a95c) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #660   +/-   ##
========================================
  Coverage    52.42%   52.42%           
========================================
  Files          151      151           
  Lines        12346    12346           
========================================
  Hits          6473     6473           
  Misses        5873     5873           
Impacted Files Coverage Δ
tests/KeyFrame.cpp 100.00% <100.00%> (ø)
tests/Point.cpp 100.00% <100.00%> (ø)

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 4e4a95c...317c0be. Read the comment docs.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Apr 19, 2021

Merging this, as it's relatively inconsequential unless used.

(Except for a switch I snuck in, to use the standard CTest variable BUILD_TESTING as the preferred way to enable/disable unit tests. It's on by default, but can be switched off to disable all unit test / coverage targets. The old DISABLE_TESTS and ENABLE_TESTS variables will be forwarded to BUILD_TESTING if they're set.)

@ferdnyc ferdnyc merged commit 0e1b8d6 into OpenShot:develop Apr 19, 2021
@ferdnyc ferdnyc deleted the individual-class-tests branch April 19, 2021 22:13
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

1 participant