test: Consolidate and update pytest options in pyproject.toml

[matthewfeickert]

Description

Resolves #1685

Remove .coveragerc and consolidate pytest options to pyproject.toml (this results in --cov-branch getting added.) In doing so, also apply the Configuring pytest recommendations from Scikit-HEP that Henry wrote.

• -ra includes a report after pytest runs with a summary on all tests except those that passed. From pytest --help:

  • -r chars show extra test summary info as specified by chars: (f)ailed, (E)rror, (s)kipped, (x)failed, (X)passed, (p)assed, (P)assed with output, (a)ll except passed (p/P), or (A)ll. (w)arnings are enabled by default (see --disable-warnings), 'N' can be used to reset the list. (default: 'fE').

  • This allows for removal of -r sx in all the CI jobs.
• -Wd enables all warnings.
  • > It adds 'd' to sys.warnoptions, which in turn adds a new first entry to _warnings.filters which matches all warnings and enables the "default" behavior, which is to show it once per execution of the Python interpreter.
• --showlocal prints locals in tracebacks.
• --strict-markers will complain if you use an unspecified fixture.
• --strict-config will raise an error if there is a mistake in the pytest config.
• log_cli_level = "info" reports INFO and above log messages on a failure.
• filterwarnings = ["error"] sets all warnings to be errors and allows for some warnings to be ignored with -W warn control syntax.
• Remove tests/__init__.py as "there’s not often a reason to make the test directory importable, and it can confuse package discovery algorithms."

To deal with the fact that the minimum supported dependencies workflow is using intentionally older releases that will have different behaviors and warnings then the rest of the tests, use the --override-ini pytest option to override the filterwarnings option with an empty list for that workflow

pytest --override-ini filterwarnings=

though also set warning control to 'default' to see the warnings.

Also remove the now unused nbQA options for black. (Amends PR #1598)

(Thanks to @alexander-held for reminding me about branch coverage in general.)

Checklist Before Requesting Reviewer

• Tests are passing
• "WIP" removed from the title of the pull request
• Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

• Summarize commit messages into a comprehensive review of the PR

* Remove .coveragerc and consolidate pytest options to pyproject.toml.
* Apply 'configuring pytest' recommendations for pytest from Scikit-HEP
(c.f. https://scikit-hep.org/developer/pytest#configuring-pytest ).
   - '-ra' includes a report after pytest runs with a summary on all tests
     except those that passed. From 'pytest --help':
     > -r chars: show extra test summary info as specified by chars: (f)ailed,
     > (E)rror, (s)kipped, (x)failed, (X)passed, (p)assed, (P)assed with output,
     > (a)ll except passed (p/P), or (A)ll. (w)arnings are enabled by default
     > (see --disable-warnings), 'N' can be used to reset the list. (default: 'fE').
   - '--showlocal' prints locals in tracebacks.
   - '--strict-markers' will complain if you use an unspecified fixture.
   - '--strict-config' will raise an error if there is a mistake in the pytest config.
   - 'log_cli_level = "info"' reports INFO and above log messages on a failure.
   - 'filterwarnings = ["error"]' sets all warnings to be errors and allows for
     some warnings to be ignored with -W warn control syntax.
     (c.f. https://docs.python.org/dev/using/cmdline.html#cmdoption-W )
* Remove `tests/__init__.py` as no reason to make the tests directory importable.
* Remove '-r sx' from pytest calls in CI jobs as pyproject.toml now applies '-ra'.
* Use 'with pytest.warns' to assert expected warnings in tests.
(c.f. https://docs.pytest.org/en/7.0.x/how-to/capture-warnings.html#warns )
* Override error on filterwarnings for the 'minimum supported dependencies' GHA workflow
as it is testing for the oldest releases that work with the latest API, not the oldest
releases that are warning free.
* Remove unused nbQA options for black from pyproject.toml.
   - Amends PR #1598

[codecov]

Codecov Report

Merging #1773 (9a42ff4) into master (b091985) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1773   +/-   ##
=======================================
  Coverage   98.12%   98.12%           
=======================================
  Files          64       64           
  Lines        4270     4270           
  Branches      683      683           
=======================================
  Hits         4190     4190           
  Misses         46       46           
  Partials       34       34           

Flag	Coverage Δ
contrib	26.25% <ø> (ø)
doctest	60.58% <ø> (ø)
unittests	96.18% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b091985...9a42ff4. Read the comment docs.

[matthewfeickert]

@kratsg @lukasheinrich Can you also run some pytest runs locally with this PR to verify that you're happy with how the new options render output now?

[henryiii]

I'd swap "-Wd enables all warnings" for filterwarnings = ["error"] (which can also have a list of allowed warnings to ignore)

[matthewfeickert]

I'd swap "-Wd enables all warnings" for filterwarnings = ["error"] (which can also have a list of allowed warnings to ignore)

Thanks for this recommendation @henryiii. Adding this has been quite insightful and is going to lead to a lot of Issues being opened!

[henryiii]

"setuptools>=30.3.0" should be "setuptools>=42"; anything before 42 was too buggy to be used with pyproject.toml (40-42), or didn't support pyproject.toml at all. (technically, PEP 517)

[matthewfeickert]

@henryiii one question. For the

pyhf/.github/workflows/lower-bound-requirements.yml

Line 1 in b091985

name: Minimum supported dependencies

workflow where we test the API against the package versions in tests/constraints.txt we don't really care if there are warnings or not, but just what is the oldest versions of our dependencies that will still run with the API.

Do you know of any way to disable filterwarnings from the command line?

pyhf/.github/workflows/lower-bound-requirements.yml

Line 38 in b091985

pytest -r sx --ignore tests/benchmarks/ --ignore tests/contrib --ignore tests/test_notebooks.py tests/

I'm a bit worried it isn't possible as pytest --help specifies that filterwarnings is "Processed after -W/--pythonwarnings".

[pytest] ini-options in the first pytest.ini|tox.ini|setup.cfg file found:

  markers (linelist):   markers for test functions
  empty_parameter_set_mark (string):
                        default marker for empty parametersets
  norecursedirs (args): directory patterns to avoid for recursion
  testpaths (args):     directories to search for tests when no files or directories are given in the command line.
  filterwarnings (linelist):
                        Each line specifies a pattern for warnings.filterwarnings. Processed after -W/--pythonwarnings.

Is there any less aggressive solution than just ripping it out?

        sed -i '/"error",/d' pyproject.toml
        pytest --ignore tests/benchmarks/ --ignore tests/contrib --ignore tests/test_notebooks.py tests/

[matthewfeickert]

Is there any less aggressive solution than just ripping it out?

After reading

$ pytest --help
...
test session debugging and configuration:
  --basetemp=dir        base temporary directory for this test run.(warning: this directory is removed if it exists)
  -V, --version         display pytest version and information about plugins. When given twice, also display information about plugins.
  -h, --help            show help message and configuration info
  -p name               early-load given plugin module name or entry point (multi-allowed).
                        To avoid loading of plugins, use the `no:` prefix, e.g. `no:doctest`.
  --trace-config        trace considerations of conftest.py files.
  --debug=[DEBUG_FILE_NAME]
                        store internal tracing debug information in this log file.
                        This file is opened with 'w' and truncated as a result, care advised.
                        Defaults to 'pytestdebug.log'.
  -o OVERRIDE_INI, --override-ini=OVERRIDE_INI
                        override ini option with "option=value" style, e.g. `-o xfail_strict=True -o cache_dir=cache`.

I see that

--override-ini filterwarnings=

works.

[alexander-held]

I understand the motivation behind filterwarnings = ["error"], I just wonder whether it is worth the extra maintenance burden to keep the list of ignores up-to date. Maybe I am overestimating the effort though, given that it seems that (at least right now) there are not a lot of things that are only due to the dependencies.

Very minor point: is there a good way to clean up the warning ignore settings periodically or automatically (if they are not used)? Assuming that something like the true_divide:RuntimeWarning would be fixed by re-designing a test (maybe even accidentally), it would probably be good to also remove that entry. When a new test will be designed in the future, the warning could be caught again and perhaps be avoided directly by re-designing the new test.

[matthewfeickert]

Maybe I am overestimating the effort though, given that it seems that (at least right now) there are not a lot of things that are only due to the depdencencies.

@alexander-held That is correct that there are quite a few things that either would get their own issue to get fixed later, like changing all instances of

json.load(open(datadir.join("abc.json")))

to

with open(datadir.join("abc.json")) as spec_file:
     spec = json.load(spec_file)

to avoid things like

pytest.PytestUnraisableExceptionWarning:Exception ignored in: <_io.FileIO [closed]>

and some of the others might be able to be dealt with, but some might not be possible to deal with easily because of the nature of hitting walls at times.

Very minor point: is there a good way to clean up the warning ignore settings periodically or automatically (if they are not used)? Assuming that something like the true_divide:RuntimeWarning would be fixed by re-designing a test (maybe even accidentally), it would probably be good to also remove that entry. When a new test will be designed in the future, the warning could be caught again and perhaps be avoided directly by re-designing the new test.

No, which is one of the reasons why ideally as many of these ignores in pyproject.toml should get their own Issues to become with pytest.warns context managers that would fail if there isn't a warning produced, like in tests/test_optim.py

    with pytest.warns(
        OptimizeWarning, match="Unknown solver options: arbitrary_option"
    ):
        assert pyhf.infer.mle.fit(data, model).tolist()

[matthewfeickert]

@kratsg @lukasheinrich I'm going to merge this to keep things going, but if there are any parts of this that you have questions on or want reverted we can iterate here or in an new Issue. 👍