Skip to content

Add query count scaling regression test for role-team-access - #1102

Merged
ptoscano merged 1 commit into
ansible:develfrom
larrymou9:larrios/team-access-query-count-test
Aug 27, 2026
Merged

Add query count scaling regression test for role-team-access#1102
ptoscano merged 1 commit into
ansible:develfrom
larrymou9:larrios/team-access-query-count-test

Conversation

@larrymou9

@larrymou9 larrymou9 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Follows up on AAP-78658. test_team_access_list_query_count mirrors the existing test_user_access_list_query_count test, since both endpoints share AccessListMixin. Confirms 0 extra queries going from 2 to 20 teams in the result — no regression here.

How to verify locally

pytest test_app/tests/rbac/api/test_access_lists.py::test_team_access_list_query_count -v

Summary by CodeRabbit

  • Tests
    • Added coverage to verify that expanding team access-list results does not cause excessive database queries.
    • Helps protect performance when displaying larger sets of teams.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e107a700-d95b-4880-87a1-29cde53963c9

📥 Commits

Reviewing files that changed from the base of the PR and between c189c6d and 57c1d10.

📒 Files selected for processing (1)
  • test_app/tests/rbac/api/test_access_lists.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The test suite now checks that team access list query counts do not grow linearly when the number of teams increases from 2 to 20.

Changes

Team access list query coverage

Layer / File(s) Summary
Query count regression test
test_app/tests/rbac/api/test_access_lists.py
Adds a test that compares query counts for 2 and 20 teams. The test requires query growth to remain below five.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 57c1d

This change adds a localized regression test for team access query scaling and introduces no production behavior changes; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: kdelee

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the added query-count scaling regression test for the role-team-access endpoint.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.77%. Comparing base (5d64aaf) to head (e955390).

@@           Coverage Diff           @@
##            devel    #1102   +/-   ##
=======================================
  Coverage   94.77%   94.77%           
=======================================
  Files         259      259           
  Lines       14485    14485           
  Branches     2218     2218           
=======================================
  Hits        13728    13728           
  Misses        757      757           
Flag Coverage Δ
py312 94.73% <ø> (ø)
py312-sqlite 94.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d64aaf...e955390. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AlanCoding AlanCoding left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So I think making 20 items in a single test to count queries is probably fine for now. But if these continue to proliferate, at some point we probably need a different strategy so it doesn't blow up our runtime and we can continue to test with increasingly larger sets and more diverse data.

@larrymou9
larrymou9 force-pushed the larrios/team-access-query-count-test branch from dd34afa to 57c1d10 Compare August 19, 2026 20:00
@larrymou9
larrymou9 force-pushed the larrios/team-access-query-count-test branch 2 times, most recently from 6751311 to ed5e1d7 Compare August 27, 2026 14:44
Follows up on AAP-78658. Mirrors the existing
test_user_access_list_query_count test since role-team-access shares
the same AccessListMixin code path. Confirms 0 extra queries going
from 2 to 20 teams in the result.

Co-authored-by: Cursor <cursoragent@cursor.com>
@larrymou9
larrymou9 force-pushed the larrios/team-access-query-count-test branch from ed5e1d7 to e955390 Compare August 27, 2026 15:02
@github-actions

Copy link
Copy Markdown

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

@ptoscano
ptoscano enabled auto-merge (squash) August 27, 2026 15:15
@sonarqubecloud

Copy link
Copy Markdown

@ptoscano
ptoscano merged commit 9ebb222 into ansible:devel Aug 27, 2026
21 of 22 checks passed
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.

3 participants