allora-network / allora-chain

Node software to run the Allora Network
https://www.allora.network/
Apache License 2.0
76 stars 71 forks source link

Update Migration Tests to include NaN Initial Regrets Test #587

Closed relyt29 closed 1 week ago

relyt29 commented 1 week ago

Purpose of Changes and their Description

In #579 @xmariachi asked for someone to write migration tests that actually check that Initial Regrets set to NaN are correctly migrated by the 0.4.1 test. I went and wrote that test, then updated the 0.4.0 tests that had been wholly copied over and not edited at all, to actually test the things 0.4.1 wants to test (in fact they were not even testing the 0.4.1 migration before, they were importing the 0.4.0 migration and testing that migation instead)

Link(s) to Ticket(s) or Issue(s) resolved by this PR

579

Are these changes tested and documented?

This PR introduces four new tests for the 0.4.1 migration: TestMigratedTopicWithNoProblems TestMigratedTopicWithNaNInitialRegret TestNotMigratedTopic and TestNotMigratedTopicWithNaNInitialRegret

Whether something was "migrated" or "not migrated" has to do with whether the topic was successfully migrated during the 0.4.0 migration.

No changes to documentation

Not necessary to edit changelog, as this is meant to be merged as part of #579.

xmariachi commented 1 week ago

❤️ FYI: My unit test request was more for the actual code cases than for the migration. Ie complex tests that go beyond the execution of one function, but more complex tests that assert conditions that last longer than a block.

Both are important ofc, but code unit tests "last" and define behavioural expectations, migration does not.

Ty for reading ;)