Skip to content

Fix: use correct pagination param in threshold filter tests#1204

Merged
mihow merged 1 commit intomainfrom
fix/flaky-threshold-test
Apr 3, 2026
Merged

Fix: use correct pagination param in threshold filter tests#1204
mihow merged 1 commit intomainfrom
fix/flaky-threshold-test

Conversation

@mihow
Copy link
Copy Markdown
Collaborator

@mihow mihow commented Apr 2, 2026

Summary

Fixes an intermittent CI test failure in TestProjectDefaultThresholdFilter.test_no_project_id_returns_all.

The API uses LimitOffsetPagination (accepts ?limit=N&offset=N), but the tests were using ?page_size=1000 which is the parameter for PageNumberPagination. The page_size param was silently ignored, causing the API to use the default PAGE_SIZE=10. When other test data was present in the DB, some test occurrences could fall off the first page of 10 results, causing intermittent assertion failures like:

AssertionError: False is not true : Missing occurrence IDs: {586}

Changed all three page_size=1000 occurrences to limit=1000 in TestProjectDefaultThresholdFilter.

Test plan

  • CI tests pass consistently (this test was previously intermittent)
  • Verify the threshold filter tests still validate the correct behavior

Summary by CodeRabbit

  • Tests
    • Updated test suite to align API request parameters with current specifications.

The API uses LimitOffsetPagination which accepts ?limit=N, not
?page_size=N. The page_size parameter was silently ignored, defaulting
to PAGE_SIZE=10. When other test data was present in the DB, some test
occurrences could fall off the first page, causing intermittent failures
in test_no_project_id_returns_all and potentially other threshold tests.

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 2, 2026 08:00
@netlify
Copy link
Copy Markdown

netlify bot commented Apr 2, 2026

Deploy Preview for antenna-ssec ready!

Name Link
🔨 Latest commit b5f139b
🔍 Latest deploy log https://app.netlify.com/projects/antenna-ssec/deploys/69ce221c9708be000820974c
😎 Deploy Preview https://deploy-preview-1204--antenna-ssec.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify bot commented Apr 2, 2026

Deploy Preview for antenna-preview canceled.

Name Link
🔨 Latest commit b5f139b
🔍 Latest deploy log https://app.netlify.com/projects/antenna-preview/deploys/69ce221cee538800085e2d0b

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ac979777-402a-4da2-8f8c-a4015588b69d

📥 Commits

Reviewing files that changed from the base of the PR and between b8133cd and b5f139b.

📒 Files selected for processing (1)
  • ami/main/tests.py

📝 Walkthrough

Walkthrough

Updated query parameter names in test URLs within ami/main/tests.py, replacing page_size=1000 with limit=1000 for API occurrences and taxa list endpoints.

Changes

Cohort / File(s) Summary
Test URL Parameter Updates
ami/main/tests.py
Updated query parameter names from page_size=1000 to limit=1000 in three test URL assertions for the occurrences and taxa list API endpoints.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A param here, a param there,
From page_size to limit we care,
The tests now hop with grace so fine,
Each API call now does align,
Our burrow code is looking bright! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description includes Summary, List of Changes (implied), and Related Issues, but is missing Detailed Description, How to Test, and Checklist sections from the template. Expand the description to include How to Test section with specific instructions, clarify testing approach, and complete the Checklist section to align with template requirements.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing pagination parameter usage in threshold filter tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/flaky-threshold-test

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates threshold filter API tests to use the correct pagination query parameter for the project’s DRF pagination setup (LimitOffsetPagination), preventing intermittent CI failures caused by default paging.

Changes:

  • Replaced page_size=1000 with limit=1000 in TestProjectDefaultThresholdFilter URLs for occurrences and taxa endpoints.
  • Ensured the “no project_id returns all” test requests a sufficiently large page size via the correct param.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mihow mihow merged commit 3d3b09f into main Apr 3, 2026
11 checks passed
@mihow mihow deleted the fix/flaky-threshold-test branch April 3, 2026 22:25
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