From 01c2ec3fcb98f13ae6cdfc6a7e4a5de72f903c6b Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Fri, 12 Mar 2021 16:00:08 -0800 Subject: [PATCH] Add ROCKSDB_GTEST_BYPASS (#8048) Summary: This is for cases that do not meet the Facebook criteria for SKIP (see new comments). Also made ROCKSDB_GTEST_{SKIP,BYPASS} print the message because gtest doesn't ever seem to. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8048 Test Plan: manual inspection of ./ribbon_test output, CI Reviewed By: mrambacher Differential Revision: D26953688 Pulled By: pdillinger fbshipit-source-id: c914eaffe7d419db6ab90a193d474531e23582e5 --- HISTORY.md | 2 +- test_util/testharness.h | 35 ++++++++++++++++++++++++++++++++--- util/ribbon_test.cc | 8 ++++---- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 9804b7230..f769af68b 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -17,7 +17,7 @@ ### New Features * Support compaction filters for the new implementation of BlobDB. Add `FilterBlobByKey()` to `CompactionFilter`. Subclasses can override this method so that compaction filters can determine whether the actual blob value has to be read during compaction. Use a new `kUndetermined` in `CompactionFilter::Decision` to indicated that further action is necessary for compaction filter to make a decision. * Add support to extend retrieval of checksums for blob files from the MANIFEST when checkpointing. During backup, rocksdb can detect corruption in blob files during file copies. -* Add an option to BackupEngine::GetBackupInfo to include the name and size of each backed-up file. Especially in the presence of file sharing among backups, this offers detailed insight into bakup space usage. +* Add an option to BackupEngine::GetBackupInfo to include the name and size of each backed-up file. Especially in the presence of file sharing among backups, this offers detailed insight into backup space usage. * Enable backward iteration on keys with user-defined timestamps. ## 6.18.0 (02/19/2021) diff --git a/test_util/testharness.h b/test_util/testharness.h index 3dcde1b6b..739f32cb9 100644 --- a/test_util/testharness.h +++ b/test_util/testharness.h @@ -15,13 +15,42 @@ #include #endif -// If GTEST_SKIP is available, use it. Otherwise, define skip as success +// A "skipped" test has a specific meaning in Facebook infrastructure: the +// test is in good shape and should be run, but something about the +// compilation or execution environment means the test cannot be run. +// Specifically, there is a hole in intended testing if any +// parameterization of a test (e.g. Foo/FooTest.Bar/42) is skipped for all +// tested build configurations/platforms/etc. +// +// If GTEST_SKIP is available, use it. Otherwise, define skip as success. +// +// The GTEST macros do not seem to print the message, even with -verbose, +// so these print to stderr. Note that these do not exit the test themselves; +// calling code should 'return' or similar from the test. #ifdef GTEST_SKIP_ -#define ROCKSDB_GTEST_SKIP(m) GTEST_SKIP_(m) +#define ROCKSDB_GTEST_SKIP(m) \ + do { \ + fputs("SKIPPED: " m "\n", stderr); \ + GTEST_SKIP_(m); \ + } while (false) /* user ; */ #else -#define ROCKSDB_GTEST_SKIP(m) GTEST_SUCCESS_("SKIPPED: " m) +#define ROCKSDB_GTEST_SKIP(m) \ + do { \ + fputs("SKIPPED: " m "\n", stderr); \ + GTEST_SUCCESS_("SKIPPED: " m); \ + } while (false) /* user ; */ #endif +// We add "bypass" as an alternative to ROCKSDB_GTEST_SKIP that is allowed to +// be a permanent condition, e.g. for intentionally omitting or disabling some +// parameterizations for some tests. (Use _DISABLED at the end of the test +// name to disable an entire test.) +#define ROCKSDB_GTEST_BYPASS(m) \ + do { \ + fputs("BYPASSED: " m "\n", stderr); \ + GTEST_SUCCESS_("BYPASSED: " m); \ + } while (false) /* user ; */ + #include #include "rocksdb/env.h" diff --git a/util/ribbon_test.cc b/util/ribbon_test.cc index 320dd26f2..ba40efcb0 100644 --- a/util/ribbon_test.cc +++ b/util/ribbon_test.cc @@ -412,7 +412,7 @@ TYPED_TEST(RibbonTypeParamTest, CompactnessAndBacktrackAndFpRate) { ROCKSDB_NAMESPACE::ribbon::BandingConfigHelper; if (sizeof(CoeffRow) < 8) { - ROCKSDB_GTEST_SKIP("Not fully supported"); + ROCKSDB_GTEST_BYPASS("Not fully supported"); return; } @@ -1124,7 +1124,7 @@ TYPED_TEST(RibbonTypeParamTest, FindOccupancy) { using KeyGen = typename TypeParam::KeyGen; if (!FLAGS_find_occ) { - fprintf(stderr, "Tool disabled during unit test runs\n"); + ROCKSDB_GTEST_BYPASS("Tool disabled during unit test runs"); return; } @@ -1238,12 +1238,12 @@ TYPED_TEST(RibbonTypeParamTest, OptimizeHomogAtScale) { using KeyGen = typename TypeParam::KeyGen; if (!FLAGS_optimize_homog) { - fprintf(stderr, "Tool disabled during unit test runs\n"); + ROCKSDB_GTEST_BYPASS("Tool disabled during unit test runs"); return; } if (!TypeParam::kHomogeneous) { - fprintf(stderr, "Only for Homogeneous Ribbon\n"); + ROCKSDB_GTEST_BYPASS("Only for Homogeneous Ribbon"); return; }