Vasilii made test_execute_primitive resilient to MIOPEN_FIND_ENFORCE

[randyspauldingamd]

Addressing a failure in https://ontrack-internal.amd.com/browse/SWDEV-524093

Fix for CPU_ExecutePrimitive_NONE* tests were sensitive to MIOPEN_FIND_ENFORCE being set in the environment. This was only affecting CQE tests.

[BrianHarrisonAMD]

Is this not already fixed by @bghimireamd's change here?

#3653

If we still have spots setting the FIND_ENFORCE environment during testing without properly clearing it, then we need to find and fix that spot I think. Otherwise, we could hit huge increases to testing runtime, and volatility in the single gtest binary.

[randyspauldingamd]

Is this not already fixed by @bghimireamd's change here?

#3653

If we still have spots setting the FIND_ENFORCE environment during testing without properly clearing it, then we need to find and fix that spot I think. Otherwise, we could hit huge increases to testing runtime, and volatility in the single gtest binary.

We're not sure. I had in my notes that I repro'd the failure last week, but I couldn't repro it today. I'm going to do some testing with and without the single binary this evening to learn more, and I've asked Bibek what has been done.

In any case, this is a good fail-safe. Increasing test robustness is one of my primary concerns.

[randyspauldingamd]

Is this not already fixed by @bghimireamd's change here?

#3653

If we still have spots setting the FIND_ENFORCE environment during testing without properly clearing it, then we need to find and fix that spot I think. Otherwise, we could hit huge increases to testing runtime, and volatility in the single gtest binary.

I've verified this crash does still exist with PR 3653. Continuing to investigate whether it might occur in CQE.

[randyspauldingamd]

Still affecting QA.
Will merge this and create followup tickets to investigate the cause.

[randyspauldingamd]

Promoted a test-local object to use for this and changed name to better reflect the usage.
Added a ticket for further investigation.

[randyspauldingamd]

The ExecutePrimitive test now follows black-box better without requiring modifications to the test itself.

[bghimireamd]

similar class is here https://github.com/ROCm/MIOpen/blob/develop/test/gtest/gtest_common.hpp#L43. Can we reuse that class?

[randyspauldingamd]

Sure, I'll add functionality to clear the var to it

accidentally approved

[reidkwja]

It might be a good idea to provide logging/exception if clearing the variable fails.

[randyspauldingamd]

I checked under the hood and I'd say the existing mechanism is sufficient. It already has strict enforcement on valid env names:

MIOpen/src/env_debug.cpp

Line 207 in 20ddfa7

MIOPEN_THROW(miopenStatusNotImplemented, "Environment variable not found: " + name);

Glad you mentioned that--I hadn't dived into this code before and it seems the env lists may have gotten out of sync.