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

Reduce pollution of system directories during test runs #57606

Merged
merged 3 commits into from
Jun 21, 2024

Conversation

strk
Copy link
Contributor

@strk strk commented May 29, 2024

Closes GH-57610
Closes GH-50587
Closes GH-48884

@github-actions github-actions bot added this to the 3.38.0 milestone May 29, 2024
@strk strk force-pushed the stop-polluting-filesystem branch 2 times, most recently from c2e9ab5 to 7079e9b Compare May 29, 2024 10:18
@strk strk added Requires Tests! Waiting on the submitter to add unit tests before eligible for merging testsuite Issue related to testsuite and removed Requires Tests! Waiting on the submitter to add unit tests before eligible for merging labels May 29, 2024
Copy link

github-actions bot commented May 29, 2024

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 605cafc)

@strk
Copy link
Contributor Author

strk commented May 29, 2024

CI reported failure is:

FAIL!  : TestQgsProject::testAsynchronousLayerLoading() Compared values are not the same
   Actual   (project->mapLayers( true ).count()): 0
   Expected (layersCount - 2)                   : 57
   Loc: [/root/QGIS/tests/src/core/testqgsproject.cpp(1098)]

https://github.com/qgis/QGIS/actions/runs/9284356490/job/25547985232?pr=57606#step:13:905

strk added 2 commits May 29, 2024 14:19
Defines a QGIS_TEST_TMP_DIR variable in top-level CMakeLists.txt
and then defines QGIS_CUSTOM_CONFIG_PATH and TMPDIR to be under
that directory for individual tests.

Closes qgisGH-50587
Closes qgisGH-48884
@strk strk changed the title Stop polluting filesystem during tests Reduce pollution of system directories during test runs May 29, 2024
@strk
Copy link
Contributor Author

strk commented May 29, 2024

For the record: this is the count of files found in the tmp/ directory after make check:

$ find tmp/ -type f | wc -l
3916

All those will now at least be under a single directory, to make their removal easier (with this PR)

@strk
Copy link
Contributor Author

strk commented May 29, 2024

The failure in this PR is due to a bug in a test which is fixed by GH-57612 -- once that is merged this can be rebased

@lnicola
Copy link
Contributor

lnicola commented Jun 10, 2024

Hey @strk, this looks like a pretty trivial PR, but nobody seems to be available to merge it.

But it doesn't look obviously wrong to me. Do you want me to file an identical PR that you could approve, working around the slow review times? It would mean bypassing the merge rules, but if nobody complains maybe that's fine.

@strk strk requested a review from rouault June 21, 2024 08:16
@strk strk merged commit cfb57fd into qgis:master Jun 21, 2024
29 checks passed
@strk strk deleted the stop-polluting-filesystem branch June 21, 2024 14:39
@nyalldawson
Copy link
Collaborator

@strk I need to revert this -- it's broken my build and causes running tests from a non-installed build to segfault.

@strk
Copy link
Contributor Author

strk commented Jun 26, 2024

@nyalldawson could you file a separate ticket and add a stacktrace ? A segfault upon specifying a custom tmpdir is most likely a missing check in a testcase

@nyalldawson
Copy link
Collaborator

@strk

nyall@nyall-framework:~/dev/build-qgis$ ctest -R GdalProvider\$ -V
UpdateCTestConfiguration  from :/home/nyall/dev/build-qgis/DartConfiguration.tcl
Parse Config file:/home/nyall/dev/build-qgis/DartConfiguration.tcl
 Add coverage exclude regular expressions.
 Add coverage exclude: /CMakeFiles/CMakeTmp/
UpdateCTestConfiguration  from :/home/nyall/dev/build-qgis/DartConfiguration.tcl
Parse Config file:/home/nyall/dev/build-qgis/DartConfiguration.tcl
Test project /home/nyall/dev/build-qgis
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added setup test logGitStatus required by fixture SOURCETREE
Added cleanup test checkGitStatus required by fixture SOURCETREE
Added 2 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 1
    Start   1: logGitStatus

1: Test command: /home/nyall/dev/QGIS/scripts/chkgitstatus.sh "log"
1: Working Directory: /home/nyall/dev/build-qgis
1: Test timeout computed to be: 1500
1/3 Test   #1: logGitStatus .....................   Passed    0.05 sec
test 496
    Start 496: PyQgsGdalProvider

496: Test command: /usr/bin/cmake "-P" "/home/nyall/dev/build-qgis/tests/src/python/PyQgsGdalProvider.cmake"
496: Working Directory: /home/nyall/dev/build-qgis/tests/src/python
496: Environment variables: 
496:  TMPDIR=/home/nyall/dev/build-qgis/tmp
496:  QGIS_CUSTOM_CONFIG_PATH=/home/nyall/dev/build-qgis/tmp/config
496: Test timeout computed to be: 1500
496: export =
496: export PYTHONPATH=/home/nyall/dev/build-qgis/output/python:/home/nyall/dev/build-qgis/output/python/plugins:/home/nyall/dev/QGIS/tests/src/python:
496: -- Running  /usr/bin/python3.12 /home/nyall/dev/QGIS/tests/src/python/test_provider_gdal.py
496: src/core/providers/qgsproviderregistry.cpp:372 : (init) [339ms] Loaded 33 providers (OAPIF;WFS;arcgisfeatureserver;arcgismapserver;arcgisvectortileservice;cesiumtiles;copc;delimitedtext;ept;gdal;gpx;grass;grassraster;mbtilesvectortiles;mdal;memory;mesh_memory;mssql;ogr;pdal;postgres;postgresraster;sensorthings;spatialite;tiledscene;vectortile;virtual;virtualraster;vpc;vtpkvectortiles;wcs;wms;xyzvectortiles) 
496: QFile::remove: Empty or null file name
496: /usr/lib64/python3.12/site-packages/osgeo/gdal.py:312: FutureWarning: Neither gdal.UseExceptions() nor gdal.DontUseExceptions() has been explicitly called. In GDAL 4.0, exceptions will be enabled by default.
496:   warnings.warn(
496: GDAL ERROR 1: Unable to compute a transformation between pixel/line and georeferenced coordinates for testInt8.tif. There is no affine transformation and no GCPs. Specify transformation option SRC_METHOD=NO_GEOTRANSFORM to bypass this check.
496: GDAL ERROR 1: Unable to compute a transformation between pixel/line and georeferenced coordinates for testInt8.tif. There is no affine transformation and no GCPs. Specify transformation option SRC_METHOD=NO_GEOTRANSFORM to bypass this check.
496: .....src/core/qgsmessagelog.cpp:39 : (logMessage) [121ms] 2024-06-27T12:57:07 WCS[1] Download of capabilities failed: Protocol "" is unknown
496: src/core/qgsmessagelog.cpp:39 : (logMessage) [0ms] 2024-06-27T12:57:07 WCS[1] Download of capabilities failed: Protocol "" is unknown
496: src/providers/wcs/qgswcscapabilities.cpp:326 : (describeCoverage) [0ms] coverage not found
496: .Could not create temporary copy
496: CMake Error at PyQgsGdalProvider.cmake:23 (message):
496:   Test failed: Subprocess aborted

@strk strk restored the stop-polluting-filesystem branch June 29, 2024 04:32
strk added a commit to strk/QGIS that referenced this pull request Jun 29, 2024
This is an amended cherry-pick of commit da11a8a
which was reverted in commit a0e6e16 by Nyall Dawson

The amendment is a fix in QString syntax as spotted by Nyall
in
qgis#57606 (review)
strk added a commit to strk/QGIS that referenced this pull request Jun 29, 2024
This is an amended cherry-pick of commit da11a8a
which was reverted in commit a0e6e16 by Nyall Dawson

The amendment is a fix in QString syntax as spotted by Nyall
in
qgis#57606 (review)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testsuite Issue related to testsuite
Projects
None yet
4 participants