MNT: move pytest.ini configs to .toml by story645 · Pull Request #27094 · matplotlib/matplotlib

story645 · 2023-10-15T04:04:11Z

PR summary

~~Fixes the pytest asserts not showing using @QuLogic 's suggestion of turning on the import-mode=importlib and~~ moves the contents of pytest.ini into pytest.toml to consolidate since we're not configuring a lot.

PR checklist

"closes #0000" is in the body of the PR description to link the related issue
new and changed code is tested
Plotting related features are demonstrated in an example
New Features and API Changes are noted with a directive and release note
Documentation complies with general and docstring guidelines

rcomer · 2023-10-16T14:35:16Z

I can reproduce the "partially initialized" errors locally against main with

python -mpytest lib/matplotlib/tests/test_basic.py --import-mode importlib

I note that CI uses python -mpytest

matplotlib/.github/workflows/tests.yml

Line 311 in 112a225

python -mpytest -raR -n auto \

Pytest docs say this about invoking that way.

I also note that the following gives me a bunch of failures that I can't begin to debug

pytest lib/matplotlib/tests/test_backends_interactive.py --import-mode importlib

story645 · 2023-10-16T16:24:49Z

Oh, do we not actually use our .toml file on CI? ☹️

ksunden · 2023-10-16T20:12:11Z

I can confirm locally that pytest ... works but python -m pytest ... fails with this branch checked out.

setting the pythonpath argument in config has no effect. (Didn't really expect it to, but hoped a little bit.)

However running as python -I -m pytest ... seems to elide that problem (-I means "run in isolated mode", ignoring both PYTHON* env vars and not adding the current directory to sys.path), thus it also escapes the "I have PYTHONPATH set" problem, which I also encountered locally (though I will make it so I can just run pytest ... because that is how I prefer it... but that much can be on me)

ksunden · 2023-10-16T20:14:52Z

note that in azure we set PYTHONFAULTHANDLER=1, and -I will ignore this, so we would also want to add -X to enable it via flag instead of envvar. (if we want to keep it)

QuLogic · 2023-10-17T01:43:35Z

Might setting -P / PYTHONSAFEPATH work here? python/cpython#31542 I don't have an environment with the new version setup to test right now.

ksunden · 2023-10-17T02:04:57Z

-P helps with the "I have an env var set" part of the problem, but not the "current directory is added to the path" part, which is what is actually causing the main part of the problem when running with python -m

QuLogic · 2023-10-17T02:17:07Z

But the latter is the point of -P...

ksunden · 2023-10-17T02:31:12Z

Ah, sorry, I had it confused with -E, so I didn't override that part and it still failed... but yeah, -I is essentially just a combination of -E and -P (well, I guess also -s, which disables user site-packages which could actually be problematic in some installs...)...

-E helps with "I have PYTHONPATH set and it fails" -P helps with "Current directory is causing it to fail"

ksunden · 2023-10-17T02:32:49Z

Somehow I get the same error missing either of those two (with PYTHONPATH set)

timhoffm · 2023-10-20T15:15:08Z

pyproject.toml

+
+[tool.pytest.ini_options]
+minversion = "7.0"
+junit_family = "xunit2"


This is the default nowadays and can thus be omitted:

https://docs.pytest.org/en/7.1.x/reference/reference.html#confval-junit_family

so is python_files =test_*py so I'm also gonna omit that
https://docs.pytest.org/en/7.1.x/reference/reference.html#confval-python_files

Not exactly.

By default, files matching test_*.py and *_test.py will be considered test modules.

We don't have any *_test.py files under lib (and only one in the whole project: galleries/examples/units/evans_test.py). So, effectively, you'll detect the same files when leaving it out.

I didn't go back in the history, but I'm pretty sure we added python_files first to avoid running evans_test.py, but later we added testpaths = lib, which excluded it in a different manner.

I'm +0.05 on leaving python_files =test_*py to be very explicit what we consider tests. (But OTOH, I likely would not have intoduced it if it was not there).

To be clear, I don't think it needs to stay either.

my bias is to not have two configurations that we think are doing the same thing cause I think that'll be harder to debug when everything breaks

QuLogic · 2023-10-20T22:45:59Z

pytest.ini

-# Because tests can be run from an installed copy, most of our Pytest
-# configuration is in the `pytest_configure` function in
-# `lib/matplotlib/testing/conftest.py`. This configuration file exists only to


I think at least the beginning of this comment is important to preserve.

Moved to .toml

remove configs that are now defaults Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

story645 added the CI: testing CI configuration and testing label Oct 15, 2023

story645 marked this pull request as draft October 15, 2023 05:38

QuLogic mentioned this pull request Oct 16, 2023

TST: Use importlib for subprocess tests #27109

Merged

1 task

timhoffm reviewed Oct 20, 2023

View reviewed changes

story645 changed the title ~~pytest configs in .toml + importlib~~ MNT: move pytest.ini configs to .toml Oct 20, 2023

story645 force-pushed the pytest branch from 3f5cf8c to fd5f000 Compare October 20, 2023 19:59

story645 added the Maintenance label Oct 20, 2023

story645 marked this pull request as ready for review October 20, 2023 20:24

timhoffm approved these changes Oct 20, 2023

View reviewed changes

QuLogic reviewed Oct 20, 2023

View reviewed changes

move pytest configs from .ini to.toml

63a11fa

remove configs that are now defaults Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>

story645 force-pushed the pytest branch from 07b2d58 to 63a11fa Compare October 22, 2023 00:24

ksunden approved these changes Oct 23, 2023

View reviewed changes

ksunden merged commit e501543 into matplotlib:main Oct 23, 2023

story645 deleted the pytest branch October 23, 2023 18:02

QuLogic added this to the v3.9.0 milestone Oct 23, 2023

Uh oh!

Conversation

story645 commented Oct 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR summary

PR checklist

Uh oh!

rcomer commented Oct 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

story645 commented Oct 16, 2023

Uh oh!

ksunden commented Oct 16, 2023

Uh oh!

ksunden commented Oct 16, 2023

Uh oh!

QuLogic commented Oct 17, 2023

Uh oh!

ksunden commented Oct 17, 2023

Uh oh!

QuLogic commented Oct 17, 2023

Uh oh!

ksunden commented Oct 17, 2023

Uh oh!

ksunden commented Oct 17, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timhoffm Oct 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

story645 commented Oct 15, 2023 •

edited

Loading

rcomer commented Oct 16, 2023 •

edited

Loading

timhoffm Oct 20, 2023 •

edited

Loading