Skip to content

Commit c3d3672

Browse files
authored
chore: Clean up toxgen (#4855)
### Description Polish toxgen a bit, improving wording, examples, formatting, removing obsolete parts. #### Issues <!-- * resolves: #1234 * resolves: LIN-1234 --> #### Reminders - Please add tests to validate your changes, and lint your code using `tox -e linters`. - Add GH Issue ID _&_ Linear ID (if applicable) - PR title should use [conventional commit](https://develop.sentry.dev/engineering-practices/commit-messages/#type) style (`feat:`, `fix:`, `ref:`, `meta:`) - For external contributors: [CONTRIBUTING.md](https://github.com/getsentry/sentry-python/blob/master/CONTRIBUTING.md), [Sentry SDK development docs](https://develop.sentry.dev/sdk/), [Discord community](https://discord.gg/Ww9hbqr)
1 parent 13a8ae1 commit c3d3672

File tree

5 files changed

+76
-107
lines changed

5 files changed

+76
-107
lines changed

scripts/populate_tox/README.md

Lines changed: 24 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ The `populate_tox.py` script fills out the auto-generated part of that template.
1616
It does this by querying PyPI for each framework's package and its metadata and
1717
then determining which versions make sense to test to get good coverage.
1818

19-
The lowest supported and latest version of a framework are always tested, with
20-
a number of releases in between:
19+
By default, the lowest supported and latest version of a framework are always
20+
tested, with a number of releases in between:
2121
- If the package has majors, we pick the highest version of each major.
2222
- If the package doesn't have multiple majors, we pick two versions in between
2323
lowest and highest.
@@ -34,7 +34,8 @@ the main package (framework, library) to test with; any additional test
3434
dependencies, optionally gated behind specific conditions; and optionally
3535
the Python versions to test on.
3636

37-
Constraints are defined using the format specified below. The following sections describe each key.
37+
Constraints are defined using the format specified below. The following sections
38+
describe each key.
3839

3940
```
4041
integration_name: {
@@ -43,7 +44,7 @@ integration_name: {
4344
rule1: [package1, package2, ...],
4445
rule2: [package3, package4, ...],
4546
},
46-
"python": python_version_specifier,
47+
"python": python_version_specifier | dict[package_version_specifier, python_version_specifier],
4748
"include": package_version_specifier,
4849
"integration_name": integration_name,
4950
"num_versions": int,
@@ -102,15 +103,14 @@ Python versions, you can say:
102103
...
103104
}
104105
```
106+
105107
This key is optional.
106108

107109
### `python`
108110

109111
Sometimes, the whole test suite should only run on specific Python versions.
110-
This can be achieved via the `python` key.
111-
112-
There are two variants how to define the Python versions to run the test suite
113-
on.
112+
This can be achieved via the `python` key. There are two variants how to define
113+
the Python versions to run the test suite on.
114114

115115
If you want the test suite to only be run on specific Python versions, you can
116116
set `python` to a version specifier. For example, if you want AIOHTTP tests to
@@ -142,18 +142,17 @@ say:
142142
The `python` key is optional, and when possible, it should be omitted. The script
143143
should automatically detect which Python versions the package supports. However,
144144
if a package has broken metadata or the SDK is explicitly not supporting some
145-
packages on specific Python versions (because of, for example, broken context
146-
vars), the `python` key can be used.
145+
packages on specific Python versions, the `python` key can be used.
147146

148147
### `include`
149148

150-
Sometimes we only want to consider testing some specific versions of packages.
151-
For example, the Starlite package has two alpha prereleases of version 2.0.0, but
152-
we do not want to test these, since Starlite 2.0 was renamed to Litestar.
149+
Sometimes we only want to test specific versions of packages. For example, the
150+
Starlite package has two alpha prereleases of version 2.0.0, but we do not want
151+
to test these, since Starlite 2.0 was renamed to Litestar.
153152

154153
The value of the `include` key expects a version specifier defining which
155154
versions should be considered for testing. For example, since we only want to test
156-
versions below 2.x in Starlite, we can use
155+
versions below 2.x in Starlite, we can use:
157156

158157
```python
159158
"starlite": {
@@ -178,13 +177,21 @@ be expressed like so:
178177

179178
Sometimes, the name of the test suite doesn't match the name of the integration.
180179
For example, we have the `openai_base` and `openai_notiktoken` test suites, both
181-
of which are actually testing the `openai` integration. If this is the case, you can use the `integration_name` key to define the name of the integration. If not provided, it will default to the name of the test suite.
180+
of which are actually testing the `openai` integration. If this is the case, you
181+
can use the `integration_name` key to define the name of the integration. If not
182+
provided, it will default to the name of the test suite.
182183

183-
Linking an integration to a test suite allows the script to access integration configuration like for example the minimum version defined in `sentry_sdk/integrations/__init__.py`.
184+
Linking an integration to a test suite allows the script to access integration
185+
configuration like, for example, the minimum supported version defined in
186+
`sentry_sdk/integrations/__init__.py`.
184187

185188
### `num_versions`
186189

187-
With this option you can tweak the default version picking behavior by specifying how many package versions should be tested. It accepts an integer equal to or greater than 2, as the oldest and latest supported versions will always be picked. Additionally, if there is a recent prerelease, it'll also always be picked (this doesn't count towards `num_versions`).
190+
With this option you can tweak the default version picking behavior by specifying
191+
how many package versions should be tested. It accepts an integer equal to or
192+
greater than 2, as the oldest and latest supported versions will always be
193+
picked. Additionally, if there is a recent prerelease, it'll also always be
194+
picked (this doesn't count towards `num_versions`).
188195

189196

190197
## How-Tos
@@ -202,26 +209,3 @@ With this option you can tweak the default version picking behavior by specifyin
202209
`scripts/split_tox_gh_actions/split_tox_gh_actions.py`.
203210
4. Add the `TESTPATH` for the test suite in `tox.jinja`'s `setenv` section.
204211
5. Run `scripts/generate-test-files.sh` and commit the changes.
205-
206-
### Migrate a test suite to populate_tox.py
207-
208-
A handful of integration test suites are still hardcoded. The goal is to migrate
209-
them all to `populate_tox.py` over time.
210-
211-
1. Remove the integration from the `IGNORE` list in `populate_tox.py`.
212-
2. Remove the hardcoded entries for the integration from the `envlist` and `deps` sections of `tox.jinja`.
213-
3. Run `scripts/generate-test-files.sh`.
214-
4. Run the test suite, either locally or by creating a PR.
215-
5. Address any test failures that happen.
216-
217-
You might have to introduce additional version bounds on the dependencies of the
218-
package. Try to determine the source of the failure and address it.
219-
220-
Common scenarios:
221-
- An old version of the tested package installs a dependency without defining
222-
an upper version bound on it. A new version of the dependency is installed that
223-
is incompatible with the package. In this case you need to determine which
224-
versions of the dependency don't contain the breaking change and restrict this
225-
in `TEST_SUITE_CONFIG`.
226-
- Tests are failing on an old Python version. In this case first double-check
227-
whether we were even testing them on that version in the original `tox.ini`.

scripts/populate_tox/populate_tox.py

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""
22
This script populates tox.ini automatically using release data from PyPI.
3+
4+
See scripts/populate_tox/README.md for more info.
35
"""
46

57
import functools
@@ -123,6 +125,7 @@ def _save_to_cache(package: str, version: Version, release: Optional[dict]) -> N
123125
releases_cache.write(json.dumps(_normalize_release(release)) + "\n")
124126

125127
CACHE[_normalize_name(package)][str(version)] = release
128+
CACHE[_normalize_name(package)][str(version)]["_accessed"] = True
126129

127130

128131
def _prefilter_releases(
@@ -150,7 +153,8 @@ def _prefilter_releases(
150153
min_supported = Version(".".join(map(str, min_supported)))
151154
else:
152155
print(
153-
f" {integration} doesn't have a minimum version defined in sentry_sdk/integrations/__init__.py. Consider defining one"
156+
f" {integration} doesn't have a minimum version defined in "
157+
f"sentry_sdk/integrations/__init__.py. Consider defining one"
154158
)
155159

156160
include_versions = None
@@ -225,7 +229,8 @@ def get_supported_releases(
225229
Get a list of releases that are currently supported by the SDK.
226230
227231
This takes into account a handful of parameters (Python support, the lowest
228-
version we've defined for the framework, the date of the release).
232+
supported version we've defined for the framework, optionally the date
233+
of the release).
229234
230235
We return the list of supported releases and optionally also the newest
231236
prerelease, if it should be tested (meaning it's for a version higher than
@@ -272,9 +277,9 @@ def pick_releases_to_test(
272277
) -> list[Version]:
273278
"""Pick a handful of releases to test from a sorted list of supported releases."""
274279
# If the package has majors (or major-like releases, even if they don't do
275-
# semver), we want to make sure we're testing them all (unless there's too
276-
# many). If not, we just pick the oldest, the newest, and a couple
277-
# in between.
280+
# semver), we want to make sure we're testing them all. If it doesn't have
281+
# multiple majors, we just pick the oldest, the newest, and a couple of
282+
# releases in between.
278283
#
279284
# If there is a relevant prerelease, also test that in addition to the above.
280285
num_versions = TEST_SUITE_CONFIG[integration].get("num_versions")
@@ -342,7 +347,7 @@ def supported_python_versions(
342347
custom_supported_versions: Optional[
343348
Union[SpecifierSet, dict[SpecifierSet, SpecifierSet]]
344349
] = None,
345-
version: Optional[Version] = None,
350+
release_version: Optional[Version] = None,
346351
) -> list[Version]:
347352
"""
348353
Get the intersection of Python versions supported by the package and the SDK.
@@ -362,6 +367,12 @@ def supported_python_versions(
362367
on Python 3.7, so we can provide this as `custom_supported_versions`. The
363368
result of this function will then by the intersection of all three, i.e.,
364369
[3.7].
370+
- The Python SDK supports Python 3.6-3.13. The package supports 3.5-3.8.
371+
Additionally, we have a limitation in place to only test this framework on
372+
Python 3.5 if the framework version is <2.0. `custom_supported_versions`
373+
will contain this restriction, and `release_version` will contain the
374+
version of the package we're currently looking at, to determine whether the
375+
<2.0 restriction applies in this case.
365376
"""
366377
supported = []
367378

@@ -377,11 +388,11 @@ def supported_python_versions(
377388
if curr in custom_supported_versions:
378389
supported.append(curr)
379390

380-
elif version is not None and isinstance(
391+
elif release_version is not None and isinstance(
381392
custom_supported_versions, dict
382393
):
383394
for v, py in custom_supported_versions.items():
384-
if version in v:
395+
if release_version in v:
385396
if curr in py:
386397
supported.append(curr)
387398
break
@@ -435,8 +446,10 @@ def _parse_python_versions_from_classifiers(classifiers: list[str]) -> list[Vers
435446

436447
def determine_python_versions(pypi_data: dict) -> Union[SpecifierSet, list[Version]]:
437448
"""
438-
Given data from PyPI's release endpoint, determine the Python versions supported by the package
439-
from the Python version classifiers, when present, or from `requires_python` if there are no classifiers.
449+
Determine the Python versions supported by the package from PyPI data.
450+
451+
We're looking at Python version classifiers, if present, and
452+
`requires_python` if there are no classifiers.
440453
"""
441454
try:
442455
classifiers = pypi_data["info"]["classifiers"]
@@ -453,7 +466,7 @@ def determine_python_versions(pypi_data: dict) -> Union[SpecifierSet, list[Versi
453466

454467
# We only use `requires_python` if there are no classifiers. This is because
455468
# `requires_python` doesn't tell us anything about the upper bound, which
456-
# depends on when the release first came out
469+
# implicitly depends on when the release first came out.
457470
try:
458471
requires_python = pypi_data["info"]["requires_python"]
459472
except (AttributeError, KeyError):
@@ -666,7 +679,8 @@ def main(fail_on_changes: bool = False) -> None:
666679
# timestamp so that we don't fail CI on a PR just because a new package
667680
# version was released, leading to unrelated changes in tox.ini.
668681
print(
669-
f"Since we're in fail_on_changes mode, we're only considering releases before the last tox.ini update at {last_updated.isoformat()}."
682+
f"Since we're in fail_on_changes mode, we're only considering "
683+
f"releases before the last tox.ini update at {last_updated.isoformat()}."
670684
)
671685

672686
global MIN_PYTHON_VERSION, MAX_PYTHON_VERSION
@@ -789,19 +803,16 @@ def main(fail_on_changes: bool = False) -> None:
789803
790804
Please don't make manual changes to `tox.ini`. Instead, make the
791805
changes to the `tox.jinja` template and/or the `populate_tox.py`
792-
script (as applicable) and regenerate the `tox.ini` file with:
793-
794-
python -m venv toxgen.env
795-
. toxgen.env/bin/activate
796-
pip install -r scripts/populate_tox/requirements.txt
797-
python scripts/populate_tox/populate_tox.py
806+
script (as applicable) and regenerate the `tox.ini` file by
807+
running scripts/generate-test-files.sh
798808
"""
799809
)
800810
)
801811
print("Done checking tox.ini. Looking good!")
802812
else:
803813
print(
804-
"Done generating tox.ini. Make sure to also update the CI YAML files to reflect the new test targets."
814+
"Done generating tox.ini. Make sure to also update the CI YAML "
815+
"files to reflect the new test targets."
805816
)
806817

807818

0 commit comments

Comments
 (0)