AntelopeIO / leap

C++ implementation of the Antelope protocol
Other
116 stars 70 forks source link

[5.0] fix eosvmoc_limits_tests on non-x86 and non-Linux (where OC is not supported) #2392

Open spoonincode opened 4 months ago

spoonincode commented 4 months ago

Recall that the way ctest tests are populated for unit_test are by finding all the .cpp files in the unittests directory, https://github.com/AntelopeIO/leap/blob/d47b35ada7b0002a4a0931ffa8c98aadc5498bd2/unittests/CMakeLists.txt#L71 sending those through a grep to find the TEST_SUITE() for each, https://github.com/AntelopeIO/leap/blob/d47b35ada7b0002a4a0931ffa8c98aadc5498bd2/unittests/CMakeLists.txt#L91 and then adding a test on what it find, for each WASM runtime enabled, https://github.com/AntelopeIO/leap/blob/d47b35ada7b0002a4a0931ffa8c98aadc5498bd2/unittests/CMakeLists.txt#L95-L96

Notice how previously the entire eosvmoc_limits_tests.cpp is #ifdefed away when OC is not enabled. The glob+grep above has no knowledge of this, so when OC is not enabled the cmake file generates a ctest entry that runs unit_test --run_test=eosvmoc_limits_tests but that will always fail because there is no test suite named eosvmoc_limits_tests.

Adjust the eosvmoc_limits test suite and tests to be defined no matter OC enabled or not, and just #ifdef out the small OC-specific part.

This change still looks strange: why are we running these OC limits tests at all with OC is not enabled for the build? Well, that's actually what is occurring even when OC is enabled normally on our x86 Linux builds. For example this previous run you can see that we run the eosvmoc_limits test even for non-OC runtimes: https://github.com/AntelopeIO/leap/actions/runs/8634314462/job/23670206127#step:5:76 It's just that when OC isn't enabled limit_violated_test() always passes. So arguably this change is more consistent with what already occurs when OC builds are enabled vs, say, stubbing out a no-op test suite and test when OC build is not enabled. Any more complex changes to how tests are populated are probably not reasonably in scope for a release branch.