Bug2611

[riv184]

PR proposes a solution to #2611 that provides a RAII class and implementation within srt/test, updates srt/test/filelist.maf, and declares the object within TEST macros and TEST_F classes.

[codecov-commenter]

Codecov Report

Merging #2680 (4fb3f04) into master (1cffd2f) will decrease coverage by 0.10%.
The diff coverage is 88.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #2680      +/-   ##
==========================================
- Coverage   67.06%   66.96%   -0.10%     
==========================================
  Files          99      100       +1     
  Lines       20149    20135      -14     
==========================================
- Hits        13512    13483      -29     
- Misses       6637     6652      +15     

Impacted Files	Coverage Δ
test/test_muxer.cpp	1.26% <0.00%> (+0.01%)	⬆️
test/test_reuseaddr.cpp	65.54% <52.94%> (ø)
test/test_bonding.cpp	98.39% <100.00%> (-0.03%)	⬇️
test/test_connection_timeout.cpp	94.73% <100.00%> (-0.07%)	⬇️
test/test_enforced_encryption.cpp	91.94% <100.00%> (-0.04%)	⬇️
test/test_env.cpp	100.00% <100.00%> (ø)
test/test_epoll.cpp	93.08% <100.00%> (-0.02%)	⬇️
test/test_fec_rebuilding.cpp	100.00% <100.00%> (ø)
test/test_file_transmission.cpp	95.69% <100.00%> (-0.05%)	⬇️
test/test_ipv6.cpp	75.55% <100.00%> (-0.27%)	⬇️
... and 9 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ethouris]

That's almost good, but I'd do it a little bit differently. As you can see, some tests do test the call to srt_startup(), some others don't, but that's not exactly intended, rather overlooked. The best would be then to use ASSERT_NE for srt_startup() and EXPECT_NE for srt_cleanup() in every case. Hence there would be something like srt::TestInitObject srtobj; declaration and it will do the calls with test macros inside. The returned value from startup may stay as a field, in case when needed.

In tests that use fixtures there's no use of it, and your solution there is actually wrong. The SetUp and TearDown functions are provided because they are called before and after every test, not at the initialization and destruction of the fixture. In tests where it is approved to make startup/cleanup calls for the whole suite in the fixture, the srt::test::InitObject class can be simply put as a field and spare the calls in ctor/dtor. For individual test startup/cleanup there's no solution that can do any better than calling them explicitly (and these tests didn't have a problem mentioned in the bug).

[riv184]

Thank you for the feedback. I will rework the approach.

[riv184]

closing this PR -

#2681 fixes Bug2611, and provides approach for consistent startup/cleanup utilizing singleton class.