From 72e81622f3fc694ba9a86381de783d7a43bbbf76 Mon Sep 17 00:00:00 2001 From: Mike Fleetwood Date: Wed, 10 May 2017 22:42:54 +0100 Subject: [PATCH] Improve diagnostics of failed BlockSpecial operator tests (#781978) Deliberately breaking one of the operator<() tests produces less than ideal diagnostics because it doesn't print the BlockSpecial objects being compared. $ ./test_BlockSpecial ... [ RUN ] BlockSpecialTest.OperatorLessThanTwoEmptyObjects test_BlockSpecial.cc:362: Failure Value of: bs1 < bs2 Actual: true Expected: false [ FAILED ] BlockSpecialTest.OperatorLessThanTwoEmptyObjects (1 ms) ... This could be solved by using the Google Test Binary Comparison assertions, however in the tests for false from (bs1 == bs2) and (bs1 < bs2) comparisons then operators != and >= would have to be implemented and the tests changed from: EXPECT_FALSE( bs1 < bs2 ); to: EXPECT_GE( bs1, bs2 ); This makes the meaning of the test less than clear. The primary purpose of the test is to check operator<(), but it is expecting the first BlockSpecial object to be GE (greater than or equal to) than the second, which is calling operator>=() which in turn is testing operator<(). For tests of the operators themselves using Google Test Binary Comparison assertions obscures what is being tested too much. Instead provide a custom failure message which prints the BlockSpecial objects failing the comparison, leaving the test still directly calling the operator being tested, like this: EXPECT_FALSE( bs1 < bs2 ) << " Where: bs1 = " << bs1 << "\n" << " And: bs2 = " << bs2; And with a suitable macro this is simplified to: EXPECT_FALSE( bs1 < bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); Now the above deliberately broken test produces this output: $ ./test_BlockSpecial ... [ RUN ] BlockSpecialTest.OperatorLessThanTwoEmptyObjects test_BlockSpecial.cc:369: Failure Value of: bs1 < bs2 Actual: true Expected: false Where: bs1 = BlockSpecial{"",0,0} And: bs2 = BlockSpecial{"",0,0} [ FAILED ] BlockSpecialTest.OperatorLessThanTwoEmptyObjects (0 ms) ... Bug 781978 - Add Google Test C++ test framework --- tests/test_BlockSpecial.cc | 43 ++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/tests/test_BlockSpecial.cc b/tests/test_BlockSpecial.cc index 3d25dad2..de9eb529 100644 --- a/tests/test_BlockSpecial.cc +++ b/tests/test_BlockSpecial.cc @@ -88,6 +88,13 @@ std::ostream& operator<<( std::ostream & out, const BlockSpecial & bs ) #define EXPECT_BSEQTUP(bs, name, major, minor) \ EXPECT_PRED_FORMAT4(CompareHelperBS2TUP, bs, name, major, minor) +// Helper to print two compared BlockSpecial objects on failure. +// Usage: +// EXPECT_TRUE( bs1 == bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); +#define ON_FAILURE_WHERE(b1, b2) \ + " Where: " << #b1 << " = " << (b1) << "\n" \ + " And: " << #b2 << " = " << (b2); + // Return block device names numbered 0 upwards like "/dev/sda" by reading entries from // /proc/partitions. static std::string get_block_name( unsigned want ) @@ -267,7 +274,7 @@ TEST( BlockSpecialTest, OperatorEqualsTwoEmptyObjects ) BlockSpecial::clear_cache(); BlockSpecial bs1; BlockSpecial bs2; - EXPECT_TRUE( bs1 == bs2 ); + EXPECT_TRUE( bs1 == bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorEqualsSamePlainFiles ) @@ -276,7 +283,7 @@ TEST( BlockSpecialTest, OperatorEqualsSamePlainFiles ) BlockSpecial::clear_cache(); BlockSpecial bs1( "/" ); BlockSpecial bs2( "/" ); - EXPECT_TRUE( bs1 == bs2 ); + EXPECT_TRUE( bs1 == bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorEqualsEmptyObjectAndPlainFile ) @@ -285,7 +292,7 @@ TEST( BlockSpecialTest, OperatorEqualsEmptyObjectAndPlainFile ) BlockSpecial::clear_cache(); BlockSpecial bs1; BlockSpecial bs2( "/" ); - EXPECT_FALSE( bs1 == bs2 ); + EXPECT_FALSE( bs1 == bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorEqualsDifferentPlainFiles ) @@ -294,7 +301,7 @@ TEST( BlockSpecialTest, OperatorEqualsDifferentPlainFiles ) BlockSpecial::clear_cache(); BlockSpecial bs1( "/" ); BlockSpecial bs2( "/proc/partitions" ); - EXPECT_FALSE( bs1 == bs2 ); + EXPECT_FALSE( bs1 == bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorEqualsSameBlockDevices ) @@ -305,7 +312,7 @@ TEST( BlockSpecialTest, OperatorEqualsSameBlockDevices ) BlockSpecial::clear_cache(); BlockSpecial bs1( bname ); BlockSpecial bs2( bname ); - EXPECT_TRUE( bs1 == bs2 ); + EXPECT_TRUE( bs1 == bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorEqualsEmptyObjectAndBlockDevice ) @@ -316,7 +323,7 @@ TEST( BlockSpecialTest, OperatorEqualsEmptyObjectAndBlockDevice ) BlockSpecial::clear_cache(); BlockSpecial bs1; BlockSpecial bs2( bname ); - EXPECT_FALSE( bs1 == bs2 ); + EXPECT_FALSE( bs1 == bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorEqualsPlainFileAndBlockDevice ) @@ -327,7 +334,7 @@ TEST( BlockSpecialTest, OperatorEqualsPlainFileAndBlockDevice ) BlockSpecial::clear_cache(); BlockSpecial bs1( "/" ); BlockSpecial bs2( bname ); - EXPECT_FALSE( bs1 == bs2 ); + EXPECT_FALSE( bs1 == bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorEqualsTwoDifferentBlockDevices ) @@ -339,7 +346,7 @@ TEST( BlockSpecialTest, OperatorEqualsTwoDifferentBlockDevices ) BlockSpecial::clear_cache(); BlockSpecial bs1( bname1 ); BlockSpecial bs2( bname2 ); - EXPECT_FALSE( bs1 == bs2 ); + EXPECT_FALSE( bs1 == bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorEqualsSameBlockDevicesWithMinorZero ) @@ -351,7 +358,7 @@ TEST( BlockSpecialTest, OperatorEqualsSameBlockDevicesWithMinorZero ) BlockSpecial::register_block_special( "/dev/dm-0", 254, 0 ); BlockSpecial bs1( "/dev/mapper/encrypted_swap" ); BlockSpecial bs2( "/dev/dm-0" ); - EXPECT_TRUE( bs1 == bs2 ); + EXPECT_TRUE( bs1 == bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorLessThanTwoEmptyObjects ) @@ -360,7 +367,7 @@ TEST( BlockSpecialTest, OperatorLessThanTwoEmptyObjects ) BlockSpecial::clear_cache(); BlockSpecial bs1; BlockSpecial bs2; - EXPECT_FALSE( bs1 < bs2 ); + EXPECT_FALSE( bs1 < bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorLessThanSamePlainFiles ) @@ -370,7 +377,7 @@ TEST( BlockSpecialTest, OperatorLessThanSamePlainFiles ) BlockSpecial::clear_cache(); BlockSpecial bs1( "/" ); BlockSpecial bs2( "/" ); - EXPECT_FALSE( bs1 < bs2 ); + EXPECT_FALSE( bs1 < bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorLessThanDifferentPlainFiles ) @@ -382,7 +389,7 @@ TEST( BlockSpecialTest, OperatorLessThanDifferentPlainFiles ) BlockSpecial::register_block_special( "/dummy_file2", 0, 0 ); BlockSpecial bs1( "/dummy_file1" ); BlockSpecial bs2( "/dummy_file2" ); - EXPECT_TRUE( bs1 < bs2 ); + EXPECT_TRUE( bs1 < bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorLessThanEmptyObjectAndPlainFile ) @@ -391,7 +398,7 @@ TEST( BlockSpecialTest, OperatorLessThanEmptyObjectAndPlainFile ) BlockSpecial::clear_cache(); BlockSpecial bs1; BlockSpecial bs2( "/" ); - EXPECT_TRUE( bs1 < bs2 ); + EXPECT_TRUE( bs1 < bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorLessThanSameBlockDevices ) @@ -403,7 +410,7 @@ TEST( BlockSpecialTest, OperatorLessThanSameBlockDevices ) BlockSpecial::clear_cache(); BlockSpecial bs1( bname ); BlockSpecial bs2( bname ); - EXPECT_FALSE( bs1 < bs2 ); + EXPECT_FALSE( bs1 < bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorLessThanEmptyObjectAndBlockDevice ) @@ -414,7 +421,7 @@ TEST( BlockSpecialTest, OperatorLessThanEmptyObjectAndBlockDevice ) BlockSpecial::clear_cache(); BlockSpecial bs1; BlockSpecial bs2( bname ); - EXPECT_TRUE( bs1 < bs2 ); + EXPECT_TRUE( bs1 < bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorLessThanPlainFileAndBlockDevice ) @@ -425,7 +432,7 @@ TEST( BlockSpecialTest, OperatorLessThanPlainFileAndBlockDevice ) BlockSpecial::clear_cache(); BlockSpecial bs1( "/" ); BlockSpecial bs2( bname ); - EXPECT_TRUE( bs1 < bs2 ); + EXPECT_TRUE( bs1 < bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorLessThanTwoDifferentBlockDevicesMajorNumbers ) @@ -437,7 +444,7 @@ TEST( BlockSpecialTest, OperatorLessThanTwoDifferentBlockDevicesMajorNumbers ) BlockSpecial::register_block_special( "/dummy_block1", 2, 0 ); BlockSpecial bs1( "/dummy_block2" ); BlockSpecial bs2( "/dummy_block1" ); - EXPECT_TRUE( bs1 < bs2 ); + EXPECT_TRUE( bs1 < bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } TEST( BlockSpecialTest, OperatorLessThanTwoDifferentBlockDevicesMinorNumbers ) @@ -449,7 +456,7 @@ TEST( BlockSpecialTest, OperatorLessThanTwoDifferentBlockDevicesMinorNumbers ) BlockSpecial::register_block_special( "/dummy_block1", 2, 1 ); BlockSpecial bs1( "/dummy_block2" ); BlockSpecial bs2( "/dummy_block1" ); - EXPECT_TRUE( bs1 < bs2 ); + EXPECT_TRUE( bs1 < bs2 ) << ON_FAILURE_WHERE( bs1, bs2 ); } } // namespace GParted