Skip to content

Move benchmarks outside pythran package#2403

Open
stratakis wants to merge 1 commit intoserge-sans-paille:masterfrom
stratakis:benchmarks
Open

Move benchmarks outside pythran package#2403
stratakis wants to merge 1 commit intoserge-sans-paille:masterfrom
stratakis:benchmarks

Conversation

@stratakis
Copy link
Copy Markdown
Contributor

@stratakis stratakis commented Feb 3, 2026

The test_ prefix of the benchmark file was making
pytest collect it and use the benchmarks directory as
root, having the tests failing.

Rename the file so it's not collected via the usual
pytest runs.

@stratakis
Copy link
Copy Markdown
Contributor Author

Discovered it while trying to update to the latest version in Fedora.

Since there was no an __init__.py file there, pytest was entering the benchmarks directory and using that as root. Placing the benchmarks outside the tests dir works without issues (although I didn't try to run the benchmark code, I assume codspeed should be utilized)

@stratakis
Copy link
Copy Markdown
Contributor Author

It seems pytest might still be collecting this file, changing to draft for a bit till I properly test it.

@stratakis stratakis marked this pull request as draft February 5, 2026 16:03
The test_ prefix of the benchmark file was making
pytest collect it and use the benchmarks directory as
root, having the tests failing.

Rename the file so it's not collected via the usual
pytest runs.
@serge-sans-paille
Copy link
Copy Markdown
Owner

ack. Just so you know I removed codspeed integration recently, so don't worry too much about integration.

I do run benchmark tests locally through

PYTHONPATH=$PWD py.test pythran/benchmarks/test_benchmark.py -v

@stratakis
Copy link
Copy Markdown
Contributor Author

Changing the name of the file so pytest doesn't collect it, hence it won't change directory seems to work. My initial assumptions of the lack of an init file was not correct, the file naming scheme was the culprit. Already tested in Fedora and tests run successfully.

@stratakis stratakis marked this pull request as ready for review February 6, 2026 01:28
@stratakis
Copy link
Copy Markdown
Contributor Author

Seems that I'm not able to change the title of the PR, but the commit message should be relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants