Skip to content

test: cover TX_TIME and TIME/ASC sort paths on v0 listing#1135

Merged
odesenfans merged 2 commits into
mainfrom
od/test-posts-legacy-sort-coverage
May 13, 2026
Merged

test: cover TX_TIME and TIME/ASC sort paths on v0 listing#1135
odesenfans merged 2 commits into
mainfrom
od/test-posts-legacy-sort-coverage

Conversation

@odesenfans
Copy link
Copy Markdown
Collaborator

Summary

Adds two black-box tests against get_matching_posts_legacy to cover sort paths that previously had no exercise:

  • test_get_matching_posts_legacy_sort_order_time: asserts both ASC and DESC orderings on the default SortBy.TIME sort.
  • test_get_matching_posts_legacy_sort_order_tx_time: seeds two chain_txs + message_confirmations rows with controlled timestamps and a third unconfirmed post, then asserts ASC/DESC orderings including the nullsfirst/nullslast branches.

Coverage on aleph.db.accessors.posts from tests/db/test_posts.py alone: 87% → 91%. The entire if sort_order: block in filter_post_select_stmt is 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

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
foxpatch-aleph previously approved these changes May 13, 2026
Copy link
Copy Markdown

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Copy link
Copy Markdown

@foxpatch-aleph foxpatch-aleph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@odesenfans odesenfans merged commit 57790d3 into main May 13, 2026
4 checks passed
@odesenfans odesenfans deleted the od/test-posts-legacy-sort-coverage branch May 13, 2026 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants