Skip to content

conftest.py: test the local west, not the installed one#883

Closed
marc-hb wants to merge 1 commit intozephyrproject-rtos:mainfrom
marc-hb:test-local
Closed

conftest.py: test the local west, not the installed one#883
marc-hb wants to merge 1 commit intozephyrproject-rtos:mainfrom
marc-hb:test-local

Conversation

@marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Nov 17, 2025

Discovered by chance after uninstall west and trying to run pytest directly. This was hopefully not a problem when running test with "uv"?

This issue was probably introduced by #859. Commit 879bb01 seems relevant too.

Discovered by chance after uninstall west and trying to run pytest directly.
This was hopefully not a problem when running test with "uv"?

This issue was probably introduced by zephyrproject-rtos#859. Commit 879bb01 seems
relevant too.

Signed-off-by: Marc Herbert <[email protected]>
@marc-hb marc-hb marked this pull request as ready for review November 17, 2025 21:07
@codecov
Copy link

codecov bot commented Nov 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.44%. Comparing base (d1a3a77) to head (b3f7f8a).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #883   +/-   ##
=======================================
  Coverage   84.44%   84.44%           
=======================================
  Files          11       11           
  Lines        3394     3394           
=======================================
  Hits         2866     2866           
  Misses        528      528           

@thorsten-klein
Copy link
Contributor

How did you run pytest? In the README it says to run `` pytest -o pythonpath=src` which actually already does this pythonpath extension. Did this not work for you?

If not, I would prefer to set this option as tools.pytest.ini_option in the pyproject.toml instead of sys.path.insert.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 17, 2025

I missed the -o pythonpath=src. After #859 I just tried pytest and it seemed to work! For a long time, until I realized this. I'm afraid people look at documentation only when things go wrong :-(

If not, I would prefer to set this option as tools.pytest.ini_option in the pyproject.toml instead of sys.path.insert.

As long as it stops people from testing (parts of?) the installed version by accident, fine by me! Would like me to do it? I'm not sure how.

@marc-hb marc-hb marked this pull request as draft November 17, 2025 22:15
@thorsten-klein
Copy link
Contributor

In PR #859, I considered adding that tools.pytest.ini_option, but I think the preferred behavior from west maintainers is to test the installed version with uv to ensure that the resulting pip package works.
I was very happy to see PR #859 accepted at all, and pytest -o pythonpath=src feels like a good compromise for switching between testing the installed version and the local worktree -- even if it is only mentioned in the README.

If more contributors run into your issue, we could consider making pythonpath=src the default.
However, in CI we should still test the installed version to ensure that the pip package works.

CC: @pdgendt

@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 18, 2025

However, in CI we should still test the installed version to ensure that the pip package works.

Agreed.

I considered adding that tools.pytest.ini_option, but I think the preferred behavior from west maintainers is to test the installed version with uv to ensure that the resulting pip package works.

So are you saying we cannot have both out of the box?

If more contributors run into your issue,...

The real, serious issue here is: people running "pytest" alone do NOT realize they're probably testing a different version. So we'll never know how many people are affected. I realized only by chance after a long time. That's why documentation is not enough in this case. It would be preferable to FAIL rather than silently testing the wrong thing. Once it fails, then everyone checks the documentation. Or asks for help; but either way they don't keep testing the wrong version.

@pdgendt
Copy link
Collaborator

pdgendt commented Nov 19, 2025

I considered adding that tools.pytest.ini_option, but I think the preferred behavior from west maintainers is to test the installed version with uv to ensure that the resulting pip package works.

So are you saying we cannot have both out of the box?

I think this is a common issue for python packages... We could use the poe tasks to our advantage here maybe, and add an argument to the test task to select the installed or local version?

If we want developers to test the local version by default, we maybe should set the ini_option, but force the installed version in CI. (we can just add a separate Github specific task like for linting/formatting)

@thorsten-klein
Copy link
Contributor

thorsten-klein commented Nov 19, 2025

I think this is a common issue for python packages... We could use the poe tasks to our advantage here maybe, and add an argument to the test task to select the installed or local version?
If we want developers to test the local version by default, we maybe should set the ini_option, but force the installed version in CI. (we can just add a separate Github specific task like for linting/formatting)

+1

@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 25, 2025

#884 looks better

@marc-hb marc-hb closed this Nov 25, 2025
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.

3 participants