Skip to content

CI(pytest): Refactor common pytest arguments to files - #5544

Merged
echoix merged 3 commits into
OSGeo:mainfrom
echoix:refactor-pytest-args
Apr 17, 2025
Merged

echoix merged 3 commits into
OSGeo:mainfrom
echoix:refactor-pytest-args

Conversation

@echoix

@echoix echoix commented Apr 13, 2025 •

Copy link
Copy Markdown
Member

Since pytest 8.2, pytest allows using a file to send an argument list (https://docs.pytest.org/en/stable/how-to/usage.html#args-from-file), using native argparse functionality (https://docs.python.org/3/library/argparse.html#fromfile-prefix-chars).

Since the different invocations of pytest in CI are repeated a couple times, and quoting rules is different in between shells (like the cmd vs powershell on Windows vs the other platforms), using argument files makes sense.
It allows to factor out duplicated arguments that needs to get updated at each place.

It is possible to use an argument file multiple times, and it is also possible to use an argument file inside an argument file, but I decided against, as for another use case, it was easier to have independent "building blocks" of common arguments when needing to not have one of them, but keep the other parts.

I had to update the pytest version on Windows, as the one provided by OSGeo4W is too old (8.1.1, it's been a little while).

TLDR: Changing pytest arguments in CI can be done once, and applied to all our files using arguments defined in separate files

@github-actions github-actions Bot added windows Microsoft Windows specific macOS macOS specific CI Continuous integration labels Apr 13, 2025
@echoix
echoix requested a review from wenzeslaus April 14, 2025 01:01
@echoix

echoix commented Apr 17, 2025 •

Copy link
Copy Markdown
Member Author

If my PRs from last week could be looked at (and merged) before the weekend, I would be ready (after rebasing), to have a small subset of gunittest-based tests that would be run by pytest.

I was expecting that the small issues fixed in separate, independent PRs, would be easy to review

@wenzeslaus wenzeslaus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unusual, but it makes sense. It generally meant for long lists by both argparse and pytest, but pytest highlights that syntax and we do have similar long list issues in GRASS (file parameter), compensating for the unusual nature of this.

@echoix

echoix commented Apr 17, 2025 •

Copy link
Copy Markdown
Member Author

My implementation of running working tests, and handling the discovery/exclusion for gunittest tests heavily uses this, and was not reasonable copying into each workflow.

@echoix
echoix merged commit 85ea9eb into OSGeo:main Apr 17, 2025
@github-actions github-actions Bot added this to the 8.5.0 milestone Apr 17, 2025
@echoix

echoix commented Apr 18, 2025

Copy link
Copy Markdown
Member Author

If you could take a look at any of my open PRs, especially in these I'd appreciate it :)

The full picture of what I want to submit after that is: echoix#404, + clean + rebase and a last fix for the difference between local osgeo4w install vs CI, or unselecting that failing test on windows

@echoix
echoix deleted the refactor-pytest-args branch September 12, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Continuous integration macOS macOS specific windows Microsoft Windows specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants