gmt icon indicating copy to clipboard operation
gmt copied to clipboard

Suddenly getting ~20 failures from master

Open PaulWessel opened this issue 2 years ago • 14 comments

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 avatar Jan 12 '24 08:01 PaulWessel

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

seisman avatar Jan 12 '24 08:01 seisman

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?

PaulWessel avatar Jan 12 '24 09:01 PaulWessel

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}

seisman avatar Jan 12 '24 10:01 seisman

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).

PaulWessel avatar Jan 12 '24 11:01 PaulWessel

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

Esteban82 avatar Jan 12 '24 11:01 Esteban82

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

I don't think it's related to #8209. See my comment at https://github.com/GenericMappingTools/gmt/issues/8209#issuecomment-1889047791.

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.

seisman avatar Jan 12 '24 12:01 seisman

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

PaulWessel avatar Jan 12 '24 12:01 PaulWessel

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?

PaulWessel avatar Jan 12 '24 17:01 PaulWessel

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.

seisman avatar Jan 14 '24 13:01 seisman

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 avatar Jan 14 '24 13:01 PaulWessel

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?

PaulWessel avatar Jan 14 '24 13:01 PaulWessel

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.

seisman avatar Jan 14 '24 13:01 seisman

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 avatar Jan 14 '24 13:01 PaulWessel

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.

PaulWessel avatar Jan 14 '24 15:01 PaulWessel

Likewise #8462?

joa-quim avatar Jul 01 '24 13:07 joa-quim