tests: fix global variable corruption, make tests execution order-independent#646
Merged
tests: fix global variable corruption, make tests execution order-independent#646
Conversation
Windows specifics: The OS more strictly locks files if they are in use, even if their usage has been completed in the code. We should not leave the komodoevents file descriptor open. TODO: The **signedmasks** descriptor is also handled during komodod / komodo-test execution and is never closed (!)
last.MoM could be modified by other tests, so we need a workaround.
IS_KOMODO_NOTARY changes critical for LegacyEvents tests.
Author
|
TODO: need to fix |
we want to output value, not address ...
Author
|
TODO: need to fix Mempool tests expects: |
prevent crash in GetMedianTimePast calculation
Author
|
TODO: Fix error with |
Author
Task for Testers:
This will help reproduce and debug any order-dependent test failures. |
Merged
static hwmheight can be non-zero after komodo_init(...) after some other tests ... so we should clear it once again.
Author
|
Found other one error: TODO:
TEST_F(TestBet, testMakePayoutCond)
{
CC *payoutCond = ebet.PayoutCond();
EXPECT_EQ("(1 of (3 of 5,5,5),(2 of (1 of 5,5,5),15))", CCShowStructure(payoutCond));
EXPECT_EQ(0, memcmp(payoutCond->subconditions[1]->subconditions[1]->code+1,
ebet.SessionTx().GetHash().begin(), 32));
} |
Author
|
TODO: |
2980f7d to
bd677f8
Compare
Author
|
Small script for tests automation: #!/bin/bash
counter=1
while true; do
echo "Now launching tests ... $counter"
output=$(./src/komodo-test --gtest_shuffle 2>&1)
ret=$?
seed=$(echo "$output" | grep -oP 'seed of \K[0-9]+')
if [ $ret -ne 0 ]; then
echo -e "\033[31mSeed: $seed\033[0m"
break
fi
counter=$((counter + 1))
done |
Author
|
So far, I’ve completed more than 1500 test iterations with randomized test order, so I believe the fixes are ready to be merged. I will also assign reviewers, but I don’t expect them to review the test logic in detail (I understand that could be difficult at this stage). Instead, I just need confirmation that Instructions on how to run the tests with randomized order are available here: #646 (comment). |
|
Caught an error here with seed 63697: Same error throw with seed 82429 |
because of path caching ...
Author
smk762
approved these changes
Mar 20, 2025
smk762
left a comment
There was a problem hiding this comment.
Confirmed previous failing seeds are now functional. Re-ran for another 1000 random seed iterations and encountered no further failing tests.
gcharang
approved these changes
Mar 21, 2025
gcharang
left a comment
There was a problem hiding this comment.
LGTM! ran the test script for more than 1700 iterations, no errors found.
TheComputerGenie
pushed a commit
to ComputerGenieCo/komodo
that referenced
this pull request
Sep 12, 2025
…ependent (GLEECBTC#646) * tests, debug: add debug printout for Global Variables * tests: fix fTxIndex set after exit from GMPArithTests.RewardsTest DeckerSU/KomodoOcean#95 (comment) * tests: fix LegacyEventsTests removing temp directory, close komodoevents Windows specifics: The OS more strictly locks files if they are in use, even if their usage has been completed in the code. We should not leave the komodoevents file descriptor open. TODO: The **signedmasks** descriptor is also handled during komodod / komodo-test execution and is never closed (!) * tests: prevent ASSETCHAINS_MAGIC corruption in ParseArgumentsTests * tests: prevent ASSETCHAINS_CCDISABLES corruption in ParseArgumentsTests * tests: move deleteIfUsedBefore to testutils.h * tests: fix TestCoinImport global variables set/unset * debug: add assetchain output stream operator<< * tests: disable GLEEC example, as an old GLEEC chain now require -datadir * tests: fix TestNotary.* tests set, now it's tied to NUM_KMD_SEASONS * tests: fix global fTxIndex state in TestChain * tests: fix TestParseNotarisation.FilePaths test * test: fix consensus params after chainparams_commandline call * tests: fix TestAlerts.AlertNotify test * tests: fix TestParseNotarisation.test_prevMoMheight tests last.MoM could be modified by other tests, so we need a workaround. * tests: fix TestParseNotarisation tests IS_KOMODO_NOTARY changes critical for LegacyEvents tests. * tests: check IS_KOMODO_NOTARY == false condition, for LegacyEventsTests * tests: fix IsInitialBlockDownload() debug output in LegacyEventsTests we want to output value, not address ... * tests: prevent ASSETCHAINS_CC corruption * tests: prevent ASSETCHAINS_COMMISSION corruption * tests: add test_block suite comments * tests: make Mempool tests independent from chainActive state prevent crash in GetMedianTimePast calculation * tests: fix TestEvalNotarisation.test_komodo_notarysinit test * test: fix TestEvalNotarisation.test_komodo_notaries test * tests: temp fix for TestNotary.KomodoNotaries test * tests: fix TestEvalNotarisation.test_komodo_notarysinit static hwmheight can be non-zero after komodo_init(...) after some other tests ... so we should clear it once again. * CC: fix CCShowStructure behavior if nullptr is passed * tests: fix all TestBet.* set (undefined behavior inside komodo_nextheight) * tests: move GTEST_COUT_COLOR to testutils.h * tests: add GetNetworkByIdStr function (id_str -> network) * tests: correct restore old network in Mempool.* and PoW.* tests * tests: fix GMPArithTests.RewardsTest (should use REGTEST) * tests: fix TestParseNotarisation.prevMoMheight test * tests: fix accessing real ~/.komodo directory instead set -datadir because of path caching ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As described here, it turns out that the success of unit test execution depended on the order in which they were run. For example, a test sequence of 1,2,3 might complete successfully, but 1,3,2 could fail. Additionally, running these tests individually, such as just 1 or just 3, showed that they worked correctly.
The root cause was that some tests modified global variables, which affected the execution of other tests. Most of the issues related to modifying global variables within the tests have now been fixed, so the results should no longer depend on the execution order.
p.s. Google Test allows you to randomize the order of test execution. To do this, you can use the command-line flag
--gtest_shuffle. If you want to be able to reproduce the test order, you can also specify the flag--gtest_random_seedwith a specific number. For example:This way, the tests will run in a random order, and by specifying a seed, you can reproduce the same order in the future.
So, unit tests should always be run with
--gtest_shuffleto ensure that all tests pass successfully, regardless of their execution order.p.p.s. Instruction for testers/reviewers: #646 (comment) .