ros / console_bridge

A ROS-independent package for logging that seamlessly pipes into rosconsole/rosout for ROS-dependent packages.
BSD 3-Clause "New" or "Revised" License
22 stars 62 forks source link

Add tests for OutputFileHandler #84

Closed brawner closed 4 years ago

brawner commented 4 years ago

It looks like OutputFileHandler was the only feature that is not currently tested in this package. This adds a test for that, and also basic stdout/stderr smoke tests.

Signed-off-by: Stephen Brawner brawner@gmail.com

codecov-commenter commented 4 years ago

Codecov Report

Merging #84 into master will increase coverage by 33.33%. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #84       +/-   ##
===========================================
+ Coverage   65.00%   98.33%   +33.33%     
===========================================
  Files           2        2               
  Lines          60       60               
===========================================
+ Hits           39       59       +20     
+ Misses         21        1       -20     
Impacted Files Coverage Δ
src/console.cpp 98.21% <0.00%> (+35.71%) :arrow_up:

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 7a64d37...63c6fea. Read the comment docs.

brawner commented 4 years ago

The StdoutStderrOutput now adds the two other lines of coverage by exercising one more path in the OutputStdHandler. 98% coverage! The only thing that remains would be mocking a failing fclose().

scpeters commented 4 years ago

this looks good, but I was just tinkering with the test and re-arranged the order of the test cases in https://github.com/scpeters/console_bridge/commit/3e5db76b88c4723256b0731d2f0e14998b761ab7 and now one of them fails on my mac

[----------] 3 tests from FileHandlerTest
[ RUN      ] FileHandlerTest.TestErrorLogs
/Users/scpeters/clone/console_bridge/test/console_TEST.cc:300: Failure
Expected: (result.find(expected_text)) != (result.npos), actual: 18446744073709551615 vs 18446744073709551615
Log file did not contain expected text, instead it contained:

: 
[  FAILED  ] FileHandlerTest.TestErrorLogs (3 ms)
[ RUN      ] FileHandlerTest.TestInformDoesntLog
[       OK ] FileHandlerTest.TestInformDoesntLog (0 ms)
[ RUN      ] FileHandlerTest.TestInformLogsWithLogLevel
[       OK ] FileHandlerTest.TestInformLogsWithLogLevel (1 ms)
[----------] 3 tests from FileHandlerTest (4 ms total)

do you have any thoughts?

brawner commented 4 years ago

Addressed initialization issue. It looks like DOH is only initialized once in the whole test file.