Skip to content

Conversation

sentrivana
Copy link
Contributor

@sentrivana sentrivana commented Sep 1, 2025

Adds supports for variants, i.e., the same test suite running with a slightly different setup (for instance, a different set of dependencies, like openai and openai_notiktoken).

To add a variant, simply add a new test suite to the config.

The tricky part is naming. I had to rename openai to openai_base since otherwise the openai_notiktoken and openai_agents test suite would be run with tox -e py<version>-openai / ./scripts/runtox.sh py<version>-openai due to how tox works. They should be treated as three different suites.

Closes #4507

Copy link

codecov bot commented Sep 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.91%. Comparing base (173df64) to head (7192946).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4730   +/-   ##
=======================================
  Coverage   84.91%   84.91%           
=======================================
  Files         156      156           
  Lines       16115    16115           
  Branches     2741     2741           
=======================================
  Hits        13684    13684           
  Misses       1644     1644           
  Partials      787      787           

@sentrivana sentrivana changed the title toxgen: Add variants toxgen: Add variants & move OpenAI under toxgen Sep 1, 2025
@sentrivana sentrivana marked this pull request as ready for review September 1, 2025 14:36
@sentrivana sentrivana requested a review from a team as a code owner September 1, 2025 14:36
@alexander-alderman-webb
Copy link
Contributor

I was looking through the checks in this PR and noticed that some did not find any targets, e.g., the 3.10 notiktoken variant found here: https://github.com/getsentry/sentry-python/actions/runs/17380359730/job/49336205814?pr=4730#logs.

@sentrivana
Copy link
Contributor Author

sentrivana commented Sep 2, 2025

I was looking through the checks in this PR and noticed that some did not find any targets, e.g., the 3.10 notiktoken variant found here: https://github.com/getsentry/sentry-python/actions/runs/17380359730/job/49336205814?pr=4730#logs.

This is expected, see

sentry-python/tox.ini

Lines 146 to 149 in 7192946

{py3.8,py3.11,py3.12}-openai_notiktoken-v1.0.1
{py3.8,py3.11,py3.12}-openai_notiktoken-v1.35.15
{py3.8,py3.11,py3.12}-openai_notiktoken-v1.69.0
{py3.8,py3.12,py3.13}-openai_notiktoken-v1.102.0
The test suite is not supposed to run against 3.10. CI will attempt to run all test suites on all Python versions, and as long as there are no targets for the specific Python version, it'll just pass. See here for how it's invoked in the GH action and the script that actually does the running/skipping.

It's this way since it was the easiest to manage/most convenient way to manage all the different test suites across all py versions. The GH actions don't have any way of knowing which Python versions they should run which test suites on, so they just run everything and we manage it on a lower level.

@sentrivana sentrivana merged commit 1d473b6 into master Sep 2, 2025
138 checks passed
@sentrivana sentrivana deleted the ivana/toxgen/variants branch September 2, 2025 08:44
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.

Add option to have variants to toxgen
2 participants