Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

autotest: run Python tests directly from source tree on Windows #9224

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

dbaston
Copy link
Member

@dbaston dbaston commented Feb 9, 2024

What does this PR do?

Modifies CMake configuration so that Python tests are run directly from the source tree, rather than from a symlink or copy in the build directory. This is an experiment in removing the following "gotcha":

(be careful on Windows, the
content of $build_dir/autotest is copied from $source_dir/autotest each
time "cmake" is run, so if you edit your test .py file directly in the
build directory, be super careful of not accidentally losing your work,
and make sure to copy its content to the source directory first. That's
admittedly an annoying point of the current test setup on Windows,
compared to Unix where we use symbolic links)

described in https://lists.osgeo.org/pipermail/gdal-dev/2024-February/058425.html

A downside of this change is that, because pytest.ini lives only in the build directory, we must always explicitly provide the config file via pytest -c pytest.ini. Is it an overall reduction in complexity? I don't know, am interested in feedback.

What are related issues/pull requests?

Tasklist

  • Review
  • Adjust for comments
  • All CI builds and checks have passed

@coveralls
Copy link
Collaborator

coveralls commented Feb 9, 2024

Coverage Status

coverage: 69.002% (+0.005%) from 68.997%
when pulling fc5fbf3 on dbaston:autotest-no-symlink
into 88b0dd8 on OSGeo:master.

@rouault
Copy link
Member

rouault commented Feb 9, 2024

ah, this was mostly me not being aware of the -c option of pytest then. But perhaps we could keep the current symlink approach on non-Windows so that only Windows users have to pay the price of using an inferior operating system ;-) ?

@dbaston dbaston changed the title autotest: run Python tests directly from source tree [WIP] autotest: run Python tests directly from source tree Apr 1, 2024
@dbaston dbaston changed the title autotest: run Python tests directly from source tree autotest: run Python tests directly from source tree on Windows Apr 1, 2024
@rouault
Copy link
Member

rouault commented Apr 17, 2024

@dbaston do we need to keep that one opened ? I'm trying to reduce the number of staled PRs to keep the list as small as possible.

@dbaston
Copy link
Member Author

dbaston commented Apr 17, 2024

I updated this so that the tests are still symlinked on Linux/Mac, so I wasn't planning more work here. It can be merged or closed if we would prefer to continue copying the tests on Windows.

Comment on lines 91 to 92
if (SKIP_COPYING_AUTOTEST_SUBDIRS)
message(STATUS "Skipping copying ${CMAKE_CURRENT_SOURCE_DIR}/${subdir}")
Copy link
Member

Choose a reason for hiding this comment

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

That can be removed. It only made sense on Windows to sometimes speed-up things to avoid repeated copying of the directories. But here we are in the branch that only symlinks.

@rouault
Copy link
Member

rouault commented Apr 17, 2024

I updated this so that the tests are still symlinked on Linux/Mac, so I wasn't planning more work here. It can be merged or closed if we would prefer to continue copying the tests on Windows.

ok, sometimes I'm getting lost in what is ready for final review/merge or staled ;-) Don't hesitate to ping me if you see a PR that should be merged

Perhaps https://gdal.org/development/testing.html#running-a-subset-of-tests-using-pytest could get some updates to explain the subtelty on Windows of doing "python -m pytest -c autotest/pytest.ini ../autotest/gcore/vrt_read.py"

@dbaston
Copy link
Member Author

dbaston commented Apr 17, 2024

Perhaps https://gdal.org/development/testing.html#running-a-subset-of-tests-using-pytest could get some updates to explain the subtelty on Windows of doing "python -m pytest -c autotest/pytest.ini ../autotest/gcore/vrt_read.py"

Good idea, I've done this at https://github.com/OSGeo/gdal/pull/9224/files#diff-1c899043c9c79440030b3ee7fcf72b357d3333d5070807d933f03ce80b4861edR51

Co-authored-by: Even Rouault <even.rouault@spatialys.com>
@rouault rouault merged commit 7539e93 into OSGeo:master Apr 18, 2024
32 checks passed
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.

None yet

3 participants