Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1] Add tox generation script, but don't use it yet #3971

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
17 changes: 17 additions & 0 deletions scripts/generate-test-files.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/bin/sh

# This script generates tox.ini and CI YAML files in one go.

set -xe

cd "$(dirname "$0")"

python -m venv .venv
. .venv/bin/activate

pip install -e ..
pip install -r populate_tox/requirements.txt
pip install -r split_tox_gh_actions/requirements.txt

python populate_tox/populate_tox.py
python split_tox_gh_actions/split_tox_gh_actions.py
136 changes: 136 additions & 0 deletions scripts/populate_tox/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
# Populate Tox

We integrate with a number of frameworks and libraries and have a test suite for
each. The tests run against different versions of the framework/library to make
sure we support everything we claim to.

This `populate_tox.py` script is responsible for picking reasonable versions to
test automatically and generating parts of `tox.ini` to capture this.

## How it works

There is a template in this directory called `tox.jinja` which contains a
combination of hardcoded and generated entries.

The `populate_tox.py` script fills out the auto-generated part of that template.
It does this by querying PyPI for each framework's package and its metadata and
then determining which versions make sense to test to get good coverage.

The lowest supported and latest version of a framework are always tested, with
a number of releases in between:
- If the package has majors, we pick the highest version of each major. For the
latest major, we also pick the lowest version in that major.
- If the package doesn't have multiple majors, we pick two versions in between
lowest and highest.

#### Caveats

- Make sure the integration name is the same everywhere. If it consists of
multiple words, use an underscore instead of a hyphen.

## Defining constraints

The `TEST_SUITE_CONFIG` dictionary defines, for each integration test suite,
the main package (framework, library) to test with; any additional test
dependencies, optionally gated behind specific conditions; and optionally
the Python versions to test on.

The format is:

```
integration_name: {
"package": name_of_main_package_on_pypi,
"deps": {
rule1: [package1, package2, ...],
rule2: [package3, package4, ...],
},
"python": python_version_specifier,
}
```

The following can be set as a rule:
- `*`: packages will be always installed
- a version specifier on the main package (e.g. `<=0.32`): packages will only
be installed if the main package falls into the version bounds specified
- specific Python version(s) in the form `py3.8,py3.9`: packages will only be
installed if the Python version matches one from the list
Copy link
Member

Choose a reason for hiding this comment

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

I find this description somewhat unclear.

The first line I would change to something like "The rule must be set to one of the following." Compared to the current first line, which is not specific regarding whether there might be some other valid values for the rule, my proposal clarifies that these are the only allowed values.

Second, I would explicitly clarify that the packages that are installed are always the ones listed under that rule.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated 474c679


Rules can be used to specify version bounds on older versions of the main
package's dependencies, for example. If e.g. Flask tests generally need
Werkzeug and don't care about its version, but Flask older than 3.0 needs
a specific Werkzeug version to work, you can say:

```
"flask": {
"deps": {
"*": ["Werkzeug"],
"<3.0": ["Werkzeug<2.1.0"],
Comment on lines +79 to +80
Copy link
Member

Choose a reason for hiding this comment

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

Does order matter here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, any order is fine

}
}
```

Sometimes, things depend on the Python version installed. If the integration
test should only run on specific Python version, e.g. if you want AIOHTTP
tests to only run on Python 3.7+, you can say:

```
"aiohttp": {
...
"python": ">=3.7",
}
```
Copy link
Member

Choose a reason for hiding this comment

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

It is strange that the snippets before and after this one use deps, but this one does not. Is this intentional? If yes, maybe reordering would make sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this was an example for the paragraph talking about the python key, specifically. I can reorganize the README a bit so that it's clearer what belongs together. I'm thinking subheadings for the different keys would already help a lot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reorganized a bit in 474c679


If, on the other hand, you need to install a specific version of a secondary
dependency on specific Python versions (so the test suite should still run on
said Python versions, just with different dependency-of-a-dependency bounds),
you can say:

```
"celery": {
...
"deps": {
"*": ["newrelic", "redis"],
"py3.7": ["importlib-metadata<5.0"],
},
},
```

## How-Tos

### Add a new test suite

1. Add the minimum supported version of the framework/library to `_MIN_VERSIONS`
in `integrations/__init__.py`. This should be the lowest version of the
framework that we can guarantee works with the SDK. If you've just added the
integration, it's fine to set this to the latest version of the framework
sentrivana marked this conversation as resolved.
Show resolved Hide resolved
at the time.
2. Add the integration and any constraints to `TEST_SUITE_CONFIG`. See the
"Defining constraints" section for the format (or copy-paste one
of the existing entries).
sentrivana marked this conversation as resolved.
Show resolved Hide resolved
3. Add the integration to one of the groups in the `GROUPS` dictionary in
`scripts/split_tox_gh_actions/split_tox_gh_actions.py`.
4. Add the `TESTPATH` for the test suite in `tox.jinja`'s `setenv` section.
5. Run `scripts/generate-test-files.sh` and commit the changes.

### Migrate a test suite to populate_tox.py

A handful of integration test suites are still hardcoded. The goal is to migrate
them all to `populate_tox.py` over time.
sentrivana marked this conversation as resolved.
Show resolved Hide resolved

1. Remove the integration from the `IGNORE` list in `populate_tox.py`.
2. Remove the hardcoded entries for the integration from the `envlist` and `deps` sections of `tox.jinja`.
2. Run `scripts/generate-test-files.sh`.
3. Run the test suite, either locally or by creating a PR.
4. Address any test failures that happen.

You might have to introduce additional version bounds on the dependencies of the
package. Try to determine the source of the failure and address it.

Common scenarios:
- An old version of the tested package installs a dependency without defining
an upper version bound on it. A new version of the dependency is installed that
is incompatible with the package. In this case you need to determine which
versions of the dependency don't contain the breaking change and restrict this
in `TEST_SUITE_CONFIG`.
- Tests are failing on an old Python version. In this case first double-check
whether we were even testing them on that version in the original `tox.ini`.
Loading
Loading