Skip to content

Conversation

ncloudioj
Copy link
Contributor

@ncloudioj ncloudioj commented Sep 23, 2025

References

JIRA: DISCO-3738

Description

My apologies for this unexpected large refactor to an integration test! Needed to upgrade httpx for a latest feature for proxying, which deprecated some APIs and broke some tests.

PR Review Checklist

Put an x in the boxes that apply

  • This PR conforms to the Contribution Guidelines
  • The PR title starts with the JIRA issue reference, format example [DISCO-####], and has the same title (if applicable)
  • [load test: (abort|skip|warn)] keywords are applied to the last commit message (if applicable)
  • Documentation has been updated (if applicable)
  • Functional and performance test coverage has been expanded and maintained (if applicable)

┆Issue is synchronized with this Jira Task

"""Test the curated recommendations endpoint response is as expected."""
async with AsyncClient(app=app, base_url="http://test") as ac:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main change – no need to create a new async client for every test (also, the app argument is deprecated in httpx), there is already an test client fixture available.

Comment on lines +542 to +544
],
indent=None,
separators=(",", ":"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't those already the default for json dumps? Curious why we're setting them explicitly, im sure im missing something 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default is (', ', ': ') with trailing space. The newer version of httpx dumps json strings in the compact mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

ohhhhh

@ncloudioj ncloudioj added this pull request to the merge queue Sep 24, 2025
Merged via the queue into main with commit baf7f6b Sep 24, 2025
15 checks passed
@ncloudioj ncloudioj deleted the disco-3738 branch September 24, 2025 19:48
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