-
Notifications
You must be signed in to change notification settings - Fork 443
Run the functests in reverse on CI #9641
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
base: main
Are you sure you want to change the base?
Conversation
731a9e3
to
86e5ff8
Compare
@@ -1,5 +1,4 @@ | |||
[tool.pytest.ini_options] | |||
addopts = "-q" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will have to be removed from the cookiecutter as well: I don't think we want pytest options in multiple places (in pyproject.toml
, in the PYTEST_ADDOPTS
envvar, as pytest
command line arguments in tox.ini
, ...). Let's keep them all in one place in the PYTEST_ADDOPTS
default value in tox.ini
. Locally, a developer can export their own PYTEST_ADDOPTS
envvar in their shell and it will override the default opts, providing a convenient way to pass custom command line opts to pytest
when just running make
or tox
commands (without having to run the pytest
command directly/manually).
@@ -85,7 +85,7 @@ commands = | |||
lint: {posargs:ruff check h tests bin} | |||
{tests,functests}: python3 -m h.scripts.init_db --delete --create | |||
tests: python -m pytest --cov --cov-report= --cov-fail-under=0 {posargs:--numprocesses logical --dist loadgroup tests/unit/} | |||
functests: python -m pytest --failed-first --new-first --no-header --quiet {posargs:tests/functional/} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will have to be updated in the cookiecutter as well.
Let's keep all the pytest options in one place in PYTEST_ADDOPTS
which can be overridden by the user, not in command line args.
I don't think we want options like --failed-first
and --new-first
as default options. These change the order that tests run in and can result in "works on my machine but not on yours" situations (it shouldn't if the tests are well written, but in practice it does happen). Also these kind of test order options are a user preference, we should just accept pytest's defaults and let the user customise them with PYTEST_ADDOPTS
if they want.
Same with --no-header
and --quiet
: these are user preferences and the dev env's defaults should just be pytest's defaults.
Also, we don't include these options in the unittests command, so why should we include them in the functests command?
We do include --cov --cov-report= --cov-fail-under=0
in the command line arguments for the unittests command above. Also --numprocesses logical --dist loadgroup
when all the tests are run at once. These ones are probably better candidates for tox.ini
to force in command line arguments: they always just need to be provided correctly (or are at least harmless when not needed) and it's less likely that users will want to customise them.
d351340
to
234923c
Compare
For speed h doesn't clear the DB between functests, see: #6845 As a defence against tests becoming dependent on data left in the DB by the tests that run before them, we ran the functests in reverse order on CI (but in normal order in dev). This was achieved by adding the `pytest-reverse` dependency and adding `--reverse` to the `pytest` command in CI. See: #7013 The `--reverse` was later accidentally removed by #8264. The cookiecutter has since been applied to h and the cookiecutter doesn't do the `--reverse` either. The cookiecutter's `ci.yml` doesn't provide a way for an individual project to add a command line argument to the `pytest` command for the functests. One solution would be to add such an option to the cookiecutter, but it's awkward and I don't like adding too many options to the cookiecutter. Another solution would be to add the `pytest-reverse` dependency and `--reverse` argument to the cookiecutter and have it be applied to all projects, and possibly also for the unittests not just the functests. This could be a good option, but [pytest-reverse](https://github.com/adamchainz/pytest-reverse) is a third-party pytest plugin not an official one, and not very popular, so perhaps we don't want to depend on it in every repo. Also, all our other projects reset the DB before each unittest or functest so they have no need for `pytest-reverse`. If we ever fix h to reset the DB before each functest we'll remove the `pytest-reverse` dependency from h as well. So this commit instead finds a clever tox way to add the `--reverse` command line option in CI but not in dev: 1. We add the `--reverse` to the `PYTEST_ADDOPTS` envvar, which is an environment variable of command line options that pytest supports as an alternative to actually including the options in the command. The cookiecutter already supports per-project customisation of the envvars in the `tox.ini` file so we can do this just for h without having to add any new options to the cookiecutter. 2. To get tox to include `--reverse` in `PYTEST_ADDOPTS` when running on CI but not when running in dev we uses tox's [interactive shell substitution](https://tox.wiki/en/3.9.0/config.html#interactive-shell-substitution) which is a feature that lets you substitute one value when tox is running in an interactive TTY (i.e. in dev) and another value when it's running non-interactively (i.e. on CI) like so: `PYTEST_ADDOPTS = {env:PYTEST_ADDOPTS:{tty::--reverse}}` (in this case the non-TTY value is an empty string). A bit clever perhaps but it gets the job done and it's a one-liner. If we ever fix h's functests to reset the DB before each test then we can remove both the `pytest-reverse` dependency and this hack.
234923c
to
6e2a38f
Compare
@@ -37,6 +37,7 @@ setenv = | |||
dev: MAILCHIMP_USER_ACTIONS_SUBACCOUNT = {env:MAILCHIMP_USER_ACTIONS_SUBACCOUNT:devdata} | |||
tests: ELASTICSEARCH_INDEX = {env:ELASTICSEARCH_INDEX:hypothesis-tests} | |||
functests: ELASTICSEARCH_INDEX = {env:ELASTICSEARCH_INDEX:hypothesis-functests} | |||
functests: PYTEST_ADDOPTS = {env:PYTEST_ADDOPTS:{tty::--reverse}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the actual one-liner hack that gets the tests to run forward in dev and backwards on CI. What this actually does:
- If
PYTEST_ADDOPTS
is set in the external environment (e.g. in the user's shell in the development environment) just use that external value. This allows development environment users to override the command line arguments used bymake functests
ortox -e functests
by settingPYTEST_ADDOPTS
. (The same also works for unittests.) - If
PYTEST_ADDOPTS
is not set in the environment and the environment is an interactive TTY, then setPYTEST_ADDOPTS
to an empty string. - If
PYTEST_ADDOPTS
is not set in the environment and the environment is not an interactive TTY, then setPYTEST_ADDOPTS
to--reverse
.
For speed h doesn't clear the DB between functests, see: #6845
As a defence against tests becoming dependent on data left in the DB by the tests that run before them, we ran the functests in reverse order on CI (but in normal order in dev). This was achieved by adding the
pytest-reverse
dependency and adding--reverse
to thepytest
command in CI. See: #7013The
--reverse
was later accidentally removed by #8264. The cookiecutter has since been applied to h and the cookiecutter doesn't do the--reverse
either.The cookiecutter's
ci.yml
doesn't provide a way for an individual project to add a command line argument to thepytest
command for the functests.One solution would be to add such an option to the cookiecutter, but it's awkward and I don't like adding too many options to the cookiecutter.
Another solution would be to add the
pytest-reverse
dependency and--reverse
argument to the cookiecutter and have it be applied to all projects, and possibly also for the unittests not just the functests. This could be a good option, but pytest-reverse is a third-party pytest plugin not an official one, and not very popular, so perhaps we don't want to depend on it in every repo. Also, all our other projects reset the DB before each unittest or functest so they have no need forpytest-reverse
. If we ever fix h to reset the DB before each functest we'll remove thepytest-reverse
dependency from h as well.So this commit instead finds a clever tox way to add the
--reverse
command line option in CI but not in dev:--reverse
to thePYTEST_ADDOPTS
envvar, which is an environment variable of command line options that pytest supports as an alternative to actually including the options in the command. The cookiecutter already supports per-project customisation of the envvars in thetox.ini
file so we can do this just for h without having to add any new options to the cookiecutter.--reverse
inPYTEST_ADDOPTS
when running on CI but not when running in dev we uses tox's interactive shell substitution which is a feature that lets you substitute one value when tox is running in an interactive TTY (i.e. in dev) and another value when it's running non-interactively (i.e. on CI) like so:PYTEST_ADDOPTS = {env:PYTEST_ADDOPTS:{tty::--reverse}}
(in this case the non-TTY value is an empty string).A bit clever perhaps but it gets the job done and it's a one-liner. If we ever fix h's functests to reset the DB before each test then we can remove both the
pytest-reverse
dependency and this hack.I've also removed some pytest options that were interfering with the order that the tests would be run in and were preventing me from seeing the test names in the output (and therefore seeing what order they were running in). These options shouldn't be in the repo anyway (see comments).
Testing
You can see that the tests are run in forwards order in dev:
And if you look at the output on CI you'll see that they're run in the reverse order: