-
-
Notifications
You must be signed in to change notification settings - Fork 360
grass.script.setup: Run the presumably failing first call internally #5914
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
base: main
Are you sure you want to change the base?
Conversation
Based on reports in OSGeo#5906, OSGeo#4480, and elsewhere, the first subprocess call to C tool fails, but the second one succeeds, so let's do the first call during the setup. This is more an exepriment, than a proposed change, but if it works...
The env changes are now in #5920. I take suggestions on what to enable in CI to get the "execute first to fail" change tested here, e.g., which tests to enable. |
Add pytest-randomly plugin on Windows (through pip instal ...), and run multiple times. It will shuffle order You could also duplicate the step that runs pytest, make it continue-on-error, and inside the same step do the cleanup so the gisrc would be cleared between iterations. Also, to speed up, set to stop on first failure like: https://docs.pytest.org/en/latest/how-to/failures.html#stopping-after-the-first-or-n-failures You would have multiple tries in a same run. |
.github/workflows/osgeo4w.yml
Outdated
- name: Run pytest with a single worker II | ||
continue-on-error: true | ||
run: | | ||
call %OSGEO4W_ROOT%\opt\grass\etc\env.bat | ||
set PYTHONPATH=%GISBASE%\etc\python;%PYTHONPATH% | ||
path %GISBASE%\lib;%GISBASE%\bin;%PATH% | ||
pytest ^ | ||
@.github/workflows/pytest_args_ci.txt ^ | ||
--maxfail=10 ^ | ||
-k "not testsuite" | ||
- name: Run pytest with a single worker III | ||
continue-on-error: true | ||
run: | | ||
call %OSGEO4W_ROOT%\opt\grass\etc\env.bat | ||
set PYTHONPATH=%GISBASE%\etc\python;%PYTHONPATH% | ||
path %GISBASE%\lib;%GISBASE%\bin;%PATH% | ||
pytest ^ | ||
@.github/workflows/pytest_args_ci.txt ^ | ||
--maxfail=10 ^ | ||
-k "not testsuite" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These were missing the shell: instruction, so didn't run.
Maybe since I didn't see anything to clear the GISRC or the hypothesis on what was the one-time change that made the rest work, only the first iteration we can conclude.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first two actually had shell. Now all of them do.
I actually wanted to follow up on the GISRC. What do you mean by cleaning it? Nothing should remain between the steps.
From the recent changes, the one which by far is the most likely change to fix I wonder if the theory about the first C executable still holds - in #5906, it was g.region outside of the create_project which failed, but nevertheless it is goes through the same shutil.which function call as g.proj in create_project. |
The g.region issue was actually directly addressed by a different fix: the env is now passed to debug which then runs g.gisenv with existing GISRC (session first) when provided as opposed to failing or working by another session lingering around in os.environ. |
Based on reports in #5906, #4480, and elsewhere, the first subprocess call to C tool fails, but the second one succeeds, so let's do the first call during the setup.
This is more an exepriment, than a proposed change, but if it works...