Skip to content

fix: compare query params order-insensitively in assert_call_count - #825

Open
Shubham-Padkonde wants to merge 3 commits into
getsentry:masterfrom
Shubham-Padkonde:fix/assert-call-count-query-param-order
Open

Shubham-Padkonde wants to merge 3 commits into
getsentry:masterfrom
Shubham-Padkonde:fix/assert-call-count-query-param-order

Conversation

@Shubham-Padkonde

@Shubham-Padkonde Shubham-Padkonde commented Sep 24, 2026 •

Copy link
Copy Markdown

assert_call_count compared the full URL string verbatim, so asserting against http://example.com/api?a=1&b=2 would fail when requests built the URL as ?b=2&a=1 (preserving the dict's insertion order).

Fix by splitting each URL into its base and a sorted list of (key, value) pairs before comparing, matching the same order-independent semantics that query_string_matcher and query_param_matcher already use.

Closes #822

Claude-Session: https://claude.ai/code/session_012acaiEt2JyZdUxjQndUWQL

What type of PR is this? (check all applicable)

  • Refactor

  • Feature

  • Bug Fix

  • Optimization

  • Documentation Update

  • Other

Description

Related Issues

  • Closes #

PR checklist

Before submitting this pull request, I have done the following:

Added/updated tests?

Current repository has 100% test coverage.

  • Yes

  • No, and this is why: <!-- _please replace this line with details on why tests

    have not been included_ -->
    
  • I need help with writing tests

Latest validation

255 tests passed. Six focused call-count tests passed after optional-URL type handling was corrected. Black, flake8, and git diff --check passed. Local mypy reports three pre-existing errors in unchanged matchers.py and test_recorder.py (also present in the saved baseline). Full tox/pre-commit matrix was not run.

assert_call_count compared the full URL string verbatim, so asserting
against http://example.com/api?a=1&b=2 would fail when requests built
the URL as ?b=2&a=1 (preserving the dict's insertion order).

Fix by splitting each URL into its base and a sorted list of (key,
value) pairs before comparing, matching the same order-independent
semantics that query_string_matcher and query_param_matcher already use.

Closes getsentry#822

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012acaiEt2JyZdUxjQndUWQL
responses.add(responses.GET, "http://example.com/api", body="ok")

# Make a request; requests preserves the insertion order of params.
requests.get("http://example.com/api", params={"b": "2", "a": "1"})

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.

It would be good to have a test case covering nested structures as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added three nested-query cases covering bracket-encoded keys, list values, reordered pairs, and percent-encoded names. The tests also ensure missing, changed, and duplicate values do not match. Included the changelog entry and corrected the optional request-URL type handling.

Validation: 255 tests passed; the six focused call-count cases passed after the type adjustment. Black, flake8, and git diff --check passed. Local mypy still reports three errors in unchanged matchers.py and test_recorder.py; no errors remain in the changed files.

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