test: cover TX_TIME and TIME/ASC sort paths on v0 listing#1135
Conversation
The v0 ``/posts/`` accessor previously had no test exercising the ``sort_by=SortBy.TX_TIME`` branch nor the ``sort_order=ASCENDING`` case. Adds two black-box tests against ``get_matching_posts_legacy``: * ``test_get_matching_posts_legacy_sort_order_time`` asserts both ASC and DESC orderings on the default TIME sort. * ``test_get_matching_posts_legacy_sort_order_tx_time`` seeds two ``chain_txs`` rows + ``message_confirmations`` entries with controlled timestamps and a third unconfirmed post, then asserts the ASC/DESC orderings including the nullsfirst/nullslast branches for posts with no confirmation. Bumps coverage on ``aleph.db.accessors.posts`` from 87% to 91%.
foxpatch-aleph
left a comment
There was a problem hiding this comment.
Clean PR that adds two black-box tests exercising the TIME and TX_TIME sort paths on the v0 listing endpoint. The tests are correctly structured, follow existing patterns in the file, and cover all branches of the sort logic in filter_post_select_stmt including the nullsfirst/nullslast paths for TX_TIME. No logic errors or security concerns found.
tests/db/test_posts.py (line 515): Minor: sort_by is not specified here, so it falls through to the else branch in filter_post_select_stmt which treats it as TIME sort. Consider passing sort_by=SortBy.TIME explicitly for clarity, or leave as-is since it tests the implicit default — either is acceptable.
mypy on CI flagged ``aleph.db.models.chains`` as not explicitly exporting ``ChainSyncProtocol``; the symbol lives in ``aleph.types.chain_sync`` and ``chains.py`` only re-imports it.
foxpatch-aleph
left a comment
There was a problem hiding this comment.
Clean PR that adds two well-structured integration tests covering previously untested sort paths (TIME and TX_TIME with ASC/DESC) in get_matching_posts_legacy. The tests are correct, follow existing patterns, and properly exercise the nullsfirst/nullslast branches for TX_TIME sorting. No bugs, security issues, or logic errors found.
Summary
Adds two black-box tests against
get_matching_posts_legacyto cover sort paths that previously had no exercise:test_get_matching_posts_legacy_sort_order_time: asserts both ASC and DESC orderings on the defaultSortBy.TIMEsort.test_get_matching_posts_legacy_sort_order_tx_time: seeds twochain_txs+message_confirmationsrows with controlled timestamps and a third unconfirmed post, then asserts ASC/DESC orderings including thenullsfirst/nullslastbranches.Coverage on
aleph.db.accessors.postsfromtests/db/test_posts.pyalone: 87% → 91%. The entireif sort_order:block infilter_post_select_stmtis now covered.Branched directly off
main(not on top of #1133) so the tests can be verified against the current code first; #1133 will rebase on this once merged.Test plan
hatch run testing:test tests/db/test_posts.py -v(11 tests pass against current main)🤖 Generated with Claude Code