fpetrogalli added inline comments.
================
Comment at: llvm/unittests/Support/Host.cpp:33
-class HostTest : public testing::Test {
- Triple Host;
-
-protected:
- bool isSupportedArchAndOS() {
- // Initially this is only testing detection of the number of
- // physical cores, which is currently only supported/tested on
- // some systems.
- return (Host.isOSWindows() && llvm_is_multithreaded()) ||
- Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
- (Host.isOSLinux() && !Host.isAndroid()) ||
- (Host.isSystemZ() && Host.isOSzOS());
- }
-
- HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
-};
-
-TEST_F(HostTest, NumPhysicalCoresSupported) {
- if (!isSupportedArchAndOS())
- GTEST_SKIP();
- int Num = sys::getHostNumPhysicalCores();
- ASSERT_GT(Num, 0);
-}
-
-TEST_F(HostTest, NumPhysicalCoresUnsupported) {
- if (isSupportedArchAndOS())
- GTEST_SKIP();
- int Num = sys::getHostNumPhysicalCores();
- ASSERT_EQ(Num, -1);
-}
+class HostTest : public testing::Test {};
----------------
Is this needed?
================
Comment at: llvm/unittests/Support/Host.cpp:401
TEST_F(HostTest, DummyRunAndGetCommandOutputUse) {
// Suppress defined-but-not-used warnings when the tests using the helper are
----------------
I think you can remove the class `HostTest` and convert the test fixture into
just `TEST(` instead of `TEST_F(`. It is kind of awkward to have this: `class
HostTest : public testing::Test {};`
================
Comment at: llvm/unittests/Support/Threading.cpp:30
+class ThreadingTest : public testing::Test {
+ Triple Host;
----------------
I don't think you need to derive from `testing:Test`? The functionality of
`isSupportedArchAndOS could be achieved just with a function.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137836/new/
https://reviews.llvm.org/D137836
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
