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

Suddenly getting ~20 failures from master #8284

Closed
PaulWessel opened this issue Jan 12, 2024 · 15 comments
Closed

Suddenly getting ~20 failures from master #8284

PaulWessel opened this issue Jan 12, 2024 · 15 comments
Assignees
Labels
bug Something isn't working documentation Improve documentation
Milestone

Comments

@PaulWessel
Copy link
Member

When we released GMT 6.5 I had no failures when running the tests. This morning, from master, I am getting about 20:

	174 - doc/scripts/GMT_tut_11.sh (Failed)
	181 - doc/scripts/GMT_tut_18.sh (Failed)
	239 - doc/examples/ex34/ex34.sh (Failed)
	328 - test/genper/east_map_8.sh (Failed)
	333 - test/genper/pacific_map_2.sh (Failed)
	434 - test/grd2cpt/equalarea.sh (Failed)
	450 - test/grdcontour/bigisland.sh (Failed)
	454 - test/grdcontour/contourlegend.sh (Failed)
	455 - test/grdcontour/contours.sh (Failed)
	470 - test/grdcontour/varpens.sh (Failed)
	496 - test/grdfft/gfilter.sh (Failed)
	500 - test/grdfill/constfill.sh (Failed)
	502 - test/grdfill/nnfill.sh (Failed)
	504 - test/grdfill/splinefill.sh (Failed)
	521 - test/grdimage/autointense.sh (Failed)
	522 - test/grdimage/autoresgrid.sh (Failed)
	531 - test/grdimage/grdcyclic.sh (Failed)
	631 - test/grdview/autointense.sh (Failed)
	682 - test/modern/imagepluscpt.sh (Failed)
	699 - test/modern/viewpluscpt.sh (Failed)

and they seem related to subtle changes in the remote relief files, but I dont think we have updated them since the release? Any thoughts @Esteban82? What happens on the CI tests, @seisman ?

@PaulWessel PaulWessel added bug Something isn't working documentation Improve documentation labels Jan 12, 2024
@PaulWessel PaulWessel added this to the 6.6.0 milestone Jan 12, 2024
@PaulWessel PaulWessel self-assigned this Jan 12, 2024
@seisman
Copy link
Member

seisman commented Jan 12, 2024

First check if you're using the static server or not.

@PaulWessel
Copy link
Member Author

Of course, forgot to do that. BUT, when I do that (call alias set-static which does

alias set-static='export GMT_DATA_SERVER=static'

so in the terminal that I launch the tests I have $GMT_DATA_SERVER as static.

my tests always used to work but not now. Adding a -Vi -Vd shows it is now trying to get the files from the server (oceania) despite this setting. In gmtest.in we have

gmtest.in:[[ -z "${GMT_DATA_SERVER}" ]] && export GMT_DATA_SERVER=@GMT_DATA_SERVER@

Should we simply set it to static there (as default - might want to run from time to time on the latest data, but so far we only use static). Of course, we stil have

ConfigDefault.cmake: set (GMT_DATA_SERVER "oceania")

and I commented this out in mine:

ConfigUserAdvanced.cmake:# set (GMT_DATA_SERVER "static")

Recommendations?

@seisman
Copy link
Member

seisman commented Jan 12, 2024

gmtest.in:[[ -z "${GMT_DATA_SERVER}" ]] && export GMT_DATA_SERVER=@GMT_DATA_SERVER@

Should we simply set it to static there (as default - might want to run from time to time on the latest data, but so far we only use static).

Do you mean changing it to:

[[ -z "${GMT_DATA_SERVER}" ]] && export GMT_DATA_SERVER=static 

or even a short version:

export GMT_DATA_SERVER=${GMT_DATA_SERVER:-static}

@PaulWessel
Copy link
Member Author

What works now for me is to leave the CMakeDefault.cmake as oceania (we have to do that for the users) and then I change my Advanced setting to static and leave gmtest.in as it was before. Now all tests pass. If I set $GMT_DATA_SERVER to oceania then I am getting the recent files and tests will fail (as intended). So seems OK as long as I do it right. We certainly want to prevent users getting into static and candidate without really knowing what they are doing (and they wont unless collaborators etc).

@Esteban82
Copy link
Member

Could be that it is the same issue that in #8209?

@seisman
Copy link
Member

seisman commented Jan 12, 2024

Could be that it is the same issue that in #8209?

I don't think it's related to #8209. See my comment at #8209 (comment).

I suspect that something wrong with [[ -z "${GMT_DATA_SERVER}" ]] && export GMT_DATA_SERVER=static which may creates new processes/shells and it may be more complicated when you run tests parallelly.

export GMT_DATA_SERVER=${GMT_DATA_SERVER:-static}

I feel this is a better solution. If GMT_DATA_SERVER is undefined, then gmtest uses static server, otherwise, gmtest uses the specified server. It also means GMT_DATA_SERVER in ConfigUser.cmake have no effects on the data server used in tests.

@PaulWessel I would suggest you applying the above change and see if everything works as you expect.

@PaulWessel
Copy link
Member Author

OK, will try in a bit once done with something else.

@PaulWessel
Copy link
Member Author

Hm, with this in gmtest.in:

export GMT_DATA_SERVER=${GMT_DATA_SERVER:-static}

This in commented out in ConfigUsersAdvanced.cmake:

# set (GMT_DATA_SERVER "static")

and this in ConfigDefault.cmake of course.

if (NOT DEFINED GMT_DATA_SERVER)
	set (GMT_DATA_SERVER "oceania")
endif (NOT DEFINED GMT_DATA_SERVER)

When I run cmake:

cd rbuild; cmake -DCMAKE_INSTALL_PREFIX=gmt6 -DCMAKE_BUILD_TYPE=Release -G Ninja ..

I get among the listings of what we find:

  • Found GMT data server : oceania

So that is the default setting. Fine. But I then do

echo $GMT_DATA_SERVER
static

and then run the tests in the rbuild directory (which is what I call the release build locally) in that terminal:

cd rbuild; ${builder} -j${ncores} check

I get ~20 or so failures because somehow the oceania in the ConfigDefault.cmake triumphs over what I set in the terminal?

@seisman
Copy link
Member

seisman commented Jan 14, 2024

I can't reproduce your issue on Linux. Are you sure you have a clean build?

I have cleaned up my ~/.gmt directory, changed gmtest.in to export GMT_DATA_SERVER=${GMT_DATA_SERVER:-static} and ran the full tests. After finishing the tests, I have ~/.gmt/static not ~/.gmt/server, which means all the tests are using the static server, not the default oceanic server.

@PaulWessel
Copy link
Member Author

OK, I once again put

export GMT_DATA_SERVER=${GMT_DATA_SERVER:-static}

into the gmtest.in and commented out the
#[[ -z "${GMT_DATA_SERVER}" ]] && export GMT_DATA_SERVER=@GMT_DATA_SERVER@
line. Rebuilding GMT now. the gmtest script now has

export GMT_DATA_SERVER=${GMT_DATA_SERVER:-static}

Then in a suitable terminal I run my set-static alias:

alias set-static='export GMT_DATA_SERVER=static'

It is now set in the terminal:

echo $GMT_DATA_SERVER
static

Now I run my tests which I do via my ctr alias that does this (rbuild is my build directory):

cd rbuild; ${builder} -j${ncores} check

Remember that the GMT defaults cmake setting has

set (GMT_DATA_SERVER "oceania")

so inside Cmake items and things created from *.in, doesn't this take precedence over whatever I set in the terminal?

Tests just finished and still 20 failures. Only thing that works for me is to activate the Advanced setting, i.e., uncomment

ConfigUserAdvanced.cmake:# set (GMT_DATA_SERVER "static")
Then things work here. But if I set oceania in my terminal it uses that instead. So must be different on Linux and macOS. I can live with this but strange that gmtest refuses to honour the environment. Maybe we need to just check if $GMT_DATA_SERVER is set in the environment and then force that?

@PaulWessel
Copy link
Member Author

CMake has an ENV operator that can be used to explore things in the environment. Maybe that Advanced setting should be something that checks ENV{GMT_DATA_SERVER} (or whatever the syntax) is and use that?

@seisman
Copy link
Member

seisman commented Jan 14, 2024

Maybe we need to just check if $GMT_DATA_SERVER is set in the environment and then force that?

CMake has an ENV operator that can be used to explore things in the environment. Maybe that Advanced setting should be something that checks ENV{GMT_DATA_SERVER} (or whatever the syntax) is and use that?

I don't think the $GMT_DATA_SERVER environment should affect the CMake settings.

@PaulWessel
Copy link
Member Author

No go. Tried this in Advanced:

if (DEFINED ENV{GMT_DATA_SERVER})
	set (GMT_DATA_SERVER "$ENV{GMT_DATA_SERVER}")
endif()

with GMT_DATA_SERVER set to static in the terminal.

@PaulWessel
Copy link
Member Author

I can live with the Advanced setting arrangement since I am rarely making new original figures for papers these days and mostly run the tests. So whatever works for you is what we should do - please do so and I will stay out of it.

@joa-quim
Copy link
Member

joa-quim commented Jul 1, 2024

Likewise #8462?

@seisman seisman closed this as completed Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improve documentation
Projects
None yet
Development

No branches or pull requests

4 participants