[llvm] 6aa8a83 - [RGT] Use GTEST_SKIP() in more places where we skip a test

Paul Robinson via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 8 15:21:01 PDT 2022


Author: Paul Robinson
Date: 2022-04-08T15:20:53-07:00
New Revision: 6aa8a836c0f60e06957b704b226547712ee752a6

URL: https://github.com/llvm/llvm-project/commit/6aa8a836c0f60e06957b704b226547712ee752a6
DIFF: https://github.com/llvm/llvm-project/commit/6aa8a836c0f60e06957b704b226547712ee752a6.diff

LOG: [RGT] Use GTEST_SKIP() in more places where we skip a test

Simply returning will report the test as PASSED when it didn't
really do anything. SKIPPED is the correct result for these.

Found by the Rotten Green Tests project.

Added: 
    

Modified: 
    llvm/unittests/Support/CrashRecoveryTest.cpp
    llvm/unittests/Support/Path.cpp
    llvm/unittests/Support/ProcessTest.cpp
    llvm/unittests/Support/ThreadPool.cpp
    llvm/unittests/Support/VirtualFileSystemTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/unittests/Support/CrashRecoveryTest.cpp b/llvm/unittests/Support/CrashRecoveryTest.cpp
index e95513eb28416..d942c9df69fb4 100644
--- a/llvm/unittests/Support/CrashRecoveryTest.cpp
+++ b/llvm/unittests/Support/CrashRecoveryTest.cpp
@@ -103,15 +103,15 @@ TEST(CrashRecoveryTest, DumpStackCleanup) {
 }
 
 TEST(CrashRecoveryTest, LimitedStackTrace) {
+  // FIXME: Handle "Depth" parameter in PrintStackTrace() function
+  // to print stack trace upto a specified Depth.
+  if (Triple(sys::getProcessTriple()).isOSWindows())
+    GTEST_SKIP();
   std::string Res;
   llvm::raw_string_ostream RawStream(Res);
   PrintStackTrace(RawStream, 1);
   std::string Str = RawStream.str();
-  // FIXME: Handle "Depth" parameter in PrintStackTrace() function
-  // to print stack trace upto a specified Depth.
-  if (!Triple(sys::getProcessTriple()).isOSWindows()) {
-    EXPECT_EQ(std::string::npos, Str.find("#1"));
-  }
+  EXPECT_EQ(std::string::npos, Str.find("#1"));
 }
 
 #ifdef _WIN32

diff  --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp
index 00c41044bda12..f609af6a0f96c 100644
--- a/llvm/unittests/Support/Path.cpp
+++ b/llvm/unittests/Support/Path.cpp
@@ -484,12 +484,12 @@ TEST(Support, HomeDirectory) {
 #endif
   // Do not try to test it if we don't know what to expect.
   // On Windows we use something better than env vars.
-  if (!expected.empty()) {
-    SmallString<128> HomeDir;
-    auto status = path::home_directory(HomeDir);
-    EXPECT_TRUE(status);
-    EXPECT_EQ(expected, HomeDir);
-  }
+  if (expected.empty())
+    GTEST_SKIP();
+  SmallString<128> HomeDir;
+  auto status = path::home_directory(HomeDir);
+  EXPECT_TRUE(status);
+  EXPECT_EQ(expected, HomeDir);
 }
 
 // Apple has their own solution for this.
@@ -564,21 +564,21 @@ TEST(Support, ConfigDirectory) {
 TEST(Support, ConfigDirectory) {
   std::string Expected = getEnvWin(L"LOCALAPPDATA");
   // Do not try to test it if we don't know what to expect.
-  if (!Expected.empty()) {
-    SmallString<128> CacheDir;
-    EXPECT_TRUE(path::user_config_directory(CacheDir));
-    EXPECT_EQ(Expected, CacheDir);
-  }
+  if (Expected.empty())
+    GTEST_SKIP();
+  SmallString<128> CacheDir;
+  EXPECT_TRUE(path::user_config_directory(CacheDir));
+  EXPECT_EQ(Expected, CacheDir);
 }
 
 TEST(Support, CacheDirectory) {
   std::string Expected = getEnvWin(L"LOCALAPPDATA");
   // Do not try to test it if we don't know what to expect.
-  if (!Expected.empty()) {
-    SmallString<128> CacheDir;
-    EXPECT_TRUE(path::cache_directory(CacheDir));
-    EXPECT_EQ(Expected, CacheDir);
-  }
+  if (Expected.empty())
+    GTEST_SKIP();
+  SmallString<128> CacheDir;
+  EXPECT_TRUE(path::cache_directory(CacheDir));
+  EXPECT_EQ(Expected, CacheDir);
 }
 #endif
 

diff  --git a/llvm/unittests/Support/ProcessTest.cpp b/llvm/unittests/Support/ProcessTest.cpp
index d4ccfe1ecfd04..4cf6ae87d8693 100644
--- a/llvm/unittests/Support/ProcessTest.cpp
+++ b/llvm/unittests/Support/ProcessTest.cpp
@@ -102,7 +102,7 @@ class PageSizeTest : public testing::Test {
 
 TEST_F(PageSizeTest, PageSize) {
   if (!isSupported())
-    return;
+    GTEST_SKIP();
 
   llvm::Expected<unsigned> Result = llvm::sys::Process::getPageSize();
   ASSERT_FALSE(!Result);

diff  --git a/llvm/unittests/Support/ThreadPool.cpp b/llvm/unittests/Support/ThreadPool.cpp
index b958b4e6dce83..b5fd81d2b18a1 100644
--- a/llvm/unittests/Support/ThreadPool.cpp
+++ b/llvm/unittests/Support/ThreadPool.cpp
@@ -83,7 +83,7 @@ class ThreadPoolTest : public testing::Test {
 #define CHECK_UNSUPPORTED()                                                    \
   do {                                                                         \
     if (isUnsupportedOSOrEnvironment())                                        \
-      return;                                                                  \
+      GTEST_SKIP();                                                            \
   } while (0);
 
 TEST_F(ThreadPoolTest, AsyncBarrier) {
@@ -263,7 +263,7 @@ TEST_F(ThreadPoolTest, AffinityMask) {
 
   // Skip this test if less than 4 threads are available.
   if (llvm::hardware_concurrency().compute_thread_count() < 4)
-    return;
+    GTEST_SKIP();
 
   using namespace llvm::sys;
   if (getenv("LLVM_THREADPOOL_AFFINITYMASK")) {
@@ -275,7 +275,7 @@ TEST_F(ThreadPoolTest, AffinityMask) {
                         [](auto &T) { return T.getData().front() < 16UL; }) &&
            "Threads ran on more CPUs than expected! The affinity mask does not "
            "seem to work.");
-    return;
+    GTEST_SKIP();
   }
   std::string Executable =
       sys::fs::getMainExecutable(TestMainArgv0, &ThreadPoolTestStringArg1);

diff  --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index 1eb7fe6b27611..9f73e89662419 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -2143,7 +2143,7 @@ TEST_F(VFSFromYAMLTest, DirectoryIteration) {
 TEST_F(VFSFromYAMLTest, DirectoryIterationSameDirMultipleEntries) {
   // https://llvm.org/bugs/show_bug.cgi?id=27725
   if (!supportsSameDirMultipleYAMLEntries())
-    return;
+    GTEST_SKIP();
 
   IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
   Lower->addDirectory("//root/zab");


        


More information about the llvm-commits mailing list