Fix the test suite - #241
Open
joelhaasnoot wants to merge 1 commit into
Open
Conversation
The suite has not run in a while. Test discovery itself was broken: openebs/tests and kv1/tests imported their modules by hand through loader.find_module(), which Python 3.12 removed, so nothing ran at all. Django discovers test*.py by itself, so the initialisers can be empty. With discovery working, 24 of 36 tests failed. Most were stale fixtures, but five were real bugs the tests had been unable to report: - Kv15Scenario.delete_all() filtered on messageendtime__gt=now, passing the function instead of calling it, so deleting a planned scenario raised TypeError - sendkv6 compared msg.delay > 0 on a nullable field in two of the three places it checks it, so a ferry message without a delay crashed the command. The init branch already guarded for None - get_pushers() raised ImproperlyConfigured when any subscriber lacked the message type, and ndovloket_rig has no KV6 endpoint - so KV6 could not even be imported. Skip subscribers that do not take the type - the KV17 journey template read "mutation > 0 or shorten > 0 and not is_recovered", and "and" binds tighter than "or", so a recover still emitted a mutation message block - verify_messages matched KV8 deletes with get_or_create() on the KV8 identity, but an updated message leaves the superseded version behind carrying that same identity, so the lookup raised MultipleObjectsReturned. Match the most recent one The rest is fixture rot: stops are matched by QuayCode rather than timingpointcode, message identity moved to OriginalMessageCode*, mutation messages moved to their own model, KV17 cancels gained showcancelledtrip and autorecover, import_rid reads bison_id, and assertEquals is gone in Python 3.12. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
The suite has not run in a long time. Test discovery itself was broken:
openebs/tests/__init__.pyandkv1/tests/__init__.pyimported their modules by hand throughloader.find_module(...).load_module(...), and Python 3.12 removedfind_module— so the whole suite died at import and zero tests ran. Django has discoveredtest*.pyon its own since 1.6, so the initialisers can simply be empty.With discovery working, 24 of 36 tests failed. 40 tests now run and pass.
Five real bugs the tests could not report
openebs/models.py:345Kv15Scenario.delete_all()filtered onmessageendtime__gt=now— the function, never called. Deleting a planned scenario raisedTypeError.ferry/.../sendkv6.py:49,69msg.delay > 0on a nullable field, so a ferry message without a delay crashed the command. The init branch at line 32 already guarded forNone; two of three places forgot.utils/views.py:88get_pushers()raisedImproperlyConfiguredwhen any subscriber lacked the message type, andndovloket_righas no KV6 endpoint — soKv6PushMixincould not even be imported. Now skips subscribers that do not take the type.templates/xml/kv17journey.xml:50mutation > 0 or shorten > 0 and not is_recovered—andbinds tighter thanor, so thenot is_recoveredguard never applied to mutation messages and a recover still emitted a mutation-message block..../verify_messages.py:99get_or_create()on the KV8 identity, but updating a message leaves the superseded version behind carrying that same identity, so the lookup raisedMultipleObjectsReturned. Now matches the most recent one.The last two change what goes out to subscribers, so they deserve a careful look. For the KV17 one the expected-output fixture already documented the intended behaviour (no mutation block on recover), which is what convinced me the precedence was accidental.
The rest is fixture rot
Tests written against behaviour that has since moved on:
QuayCodenow, nottimingpointcode, and message identity moved toOriginalMessageCode*— the KV8 rows intest_command_verifypredate bothKv17MutationMessagemodel; the tests still builtKv17StopChange(type=5), which no longer feeds the templateshowcancelledtripandautorecover(Features showcancelled & autorecover #214) — added to the expected XMLimport_ridreadsbison_idandquaycoderef; the test CSV still usedoperator_idand omitted the quay columnSUBREASONTYPEgained an entry, soSUBREASONTYPE[1][0]no longer matched the literal in the expected XML — pinned to the literal insteadassertEqualsandunittest.testare gone in Python 3.12Testing
manage.py test→ 40 tests, OK, run twice. Also re-ran the app smoke checks afterwards, since this touches the KV17 template and three commands: login plus/bericht,/kaart,/haltes.geojson,/stop/search.json,/scenario,/ritaanpassingand/admin/all still return 200.🤖 Generated with Claude Code