[gTests]Fix GTEST_SKIP kills all gtests in miopen_gtest

[xinlipn]

miopen_gtest wouldn't start on e.g. Navi3x as below. This is because GTEST_SKIP() is invoked in a global environment setup. The fix is to skip the test in TEST_P(). Log with the fix is attached at the bottom.

root@aus-navi3x-09:/MIOpen/build/bin# ./miopen_gtest
Running main() from /opt/rocm/cget/build/tmp-c4f1a42bcee536579a1248d25169f834201170c187fc6d80e8c510a7d28f6f48/googletest-1.14.0/googletest/src/gtest_main .cc
/MIOpen/test/gtest/api_convbiasactiv.cpp:189: Skipped
Skipping fusion test on unsupported ASIC

PRNG seed: 12345678
[==========] Running 8939 tests from 394 test suites.
[----------] Global test environment set-up.
Skipping fusion test on unsupported ASIC

[----------] Global test environment tear-down
[==========] 8939 tests from 394 test suites ran. (0 ms total)
[ PASSED ] 8939 tests.

YOU HAVE 133 DISABLED TESTS

Log with the fix on Navi3x
navi3x.log

[BrianHarrisonAMD]

Can we move the skip to the start of SetUp(), and use the device flags for skipping instead?

void SetUp() override
{
    if(!IsTestSupportedForDevice())
    {
         GTEST_SKIP();
    }

   // remaining Setup
}



bool IsTestSupportedForDevice()
{
#if WORKAROUND_ISSUE_2212
    using namespace miopen::debug;
    using e_mask = enabled<Gpu::gfx900, Gpu::gfx906, Gpu::gfx908, Gpu::gfx90A, Gpu::gfx103X>;
    using d_mask = disabled<Gpu::gfx110X, Gpu::gfx94X>;
   return ::IsTestSupportedForDevMask<d_mask, e_mask>();
#else
    return true;
#endif
}

[CAHEK7]

It's a part of overall gtest improvements https://github.com/ROCm/MIOpen/wiki/GTest-development#early-skip
Probably we should add some tickets here #3140

[xinlipn]

Can we move the skip to the start of SetUp(), and use the device flags for skipping instead?

void SetUp() override
{
    if(!IsTestSupportedForDevice())
    {
         GTEST_SKIP();
    }

   // remaining Setup
}



bool IsTestSupportedForDevice()
{
#if WORKAROUND_ISSUE_2212
    using namespace miopen::debug;
    using e_mask = enabled<Gpu::gfx900, Gpu::gfx906, Gpu::gfx908, Gpu::gfx90A, Gpu::gfx103X>;
    using d_mask = disabled<Gpu::gfx110X, Gpu::gfx94X>;
   return ::IsTestSupportedForDevMask<d_mask, e_mask>();
#else
    return true;
#endif
}

@BrianHarrisonAMD , thanks for the comments. Code updated

[BrianHarrisonAMD]

I think we could combine these into a single method that returns the testcases, and I think the comment isn't applicable anymore with this fix.

std::vector<CBATestCase> GetTestValues()
{
    return {{ 16, 128, 16, 16, 128, 3, 3, 0, 0, 1, 1, 1, 1, miopenActivationRELU, miopenConvolution }};
}

[xinlipn]

I think we could combine these into a single method that returns the testcases, and I think the comment isn't applicable anymore with this fix.

std::vector<CBATestCase> GetTestValues()
{
    return {{ 16, 128, 16, 16, 128, 3, 3, 0, 0, 1, 1, 1, 1, miopenActivationRELU, miopenConvolution }};
}

@BrianHarrisonAMD , thanks for the comments. Cleaned up code

[BrianHarrisonAMD]

Couple minor comments, but looks good!

Interesting find!
I had no idea it worked that way hehe.

[atamazov]

🟢 LGTM