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

Move all long2short (l2s) tests into a separate test directory? #8431

Closed
seisman opened this issue Apr 8, 2024 · 8 comments
Closed

Move all long2short (l2s) tests into a separate test directory? #8431

seisman opened this issue Apr 8, 2024 · 8 comments

Comments

@seisman
Copy link
Member

seisman commented Apr 8, 2024

We have more than 100 l2s tests, which are tests for translating long-form options to short-form options. Currently, these l2s tests are located in the subdirectory for each module.

$ ls test/**/*l2s*
test/batch/batch-l2s.sh                   test/grdsample/grdsample-l2s.sh
test/begin/begin-l2s.sh                   test/grdselect/grdselect-l2s.sh
test/blockmean/blockmean-l2s.sh           test/grdtrack/grdtrack-l2s.sh
test/blockmedian/blockmedian-l2s.sh       test/grdtrend/grdtrend-l2s.sh
test/blockmode/blockmode-l2s.sh           test/grdvector/grdvector-l2s.sh
test/common/common-l2s.sh                 test/grdview/grdview-l2s.sh
test/dimfilter/dimfilter-l2s.sh           test/grdvolume/grdvolume-l2s.sh
test/filter1d/filter1d-l2s.sh             test/greenspline/greenspline-l2s.sh
test/fitcircle/fitcircle-l2s.sh           test/inset/inset-l2s.sh
test/gmt2kml/gmt2kml-l2s.sh               test/kml2gmt/kml2gmt-l2s.sh
test/gmtbinstats/gmtbinstats-l2s.sh       test/makecpt/makecpt-l2s.sh
test/gmtconnect/gmtconnect-l2s.sh         test/mapproject/mapproject-l2s.sh
test/gmtconvert/gmtconvert-l2s.sh         test/movie/movie-l2s.sh
test/gmtdefaults/gmtdefaults-l2s.sh       test/nearneighbor/nearneighbor-l2s.sh
test/gmtget/gmtget-l2s.sh                 test/project/project-l2s.sh
test/gmtinfo/gmtinfo-l2s.sh               test/psbarb/psbarb-l2s.sh
test/gmtlogo/gmtlogo-l2s.sh               test/psbasemap/psbasemap-l2s.sh
test/gmtmath/gmtmath-l2s.sh               test/psclip/psclip-l2s.sh
test/gmtregress/gmtregress-l2s.sh         test/pscoast/pscoast-l2s.sh
test/gmtselect/gmtselect-l2s.sh           test/pscontour/pscontour-l2s.sh
test/gmtset/gmtset-l2s.sh                 test/psconvert/psconvert-l2s.sh
test/gmtsimplify/gmtsimplify-l2s.sh       test/psevents/psevents-l2s.sh
test/gmtspatial/gmtspatial-l2s.sh         test/pshistogram/pshistogram-l2s.sh
test/gmtsplit/gmtsplit-l2s.sh             test/psimage/psimage-l2s.sh
test/gmtvector/gmtvector-l2s.sh           test/pslegend/pslegend-l2s.sh
test/gmtwhich/gmtwhich-l2s.sh             test/psmask/psmask-l2s.sh
test/grd2cpt/grd2cpt-l2s.sh               test/pspolar/pspolar-l2s.sh
test/grd2kml/grd2kml-l2s.sh               test/psrose/psrose-l2s.sh
test/grd2xyz/grd2xyz-l2s.sh               test/pssac/pssac-l2s.sh
test/grdbarb/grdbarb-l2s.sh               test/psscale/psscale-l2s.sh
test/grdblend/grdblend-l2s.sh             test/pssolar/pssolar-l2s.sh
test/grdclip/grdclip-l2s.sh               test/psternary/psternary-l2s.sh
test/grdcontour/grdcontour-l2s.sh         test/pstext/pstext-l2s.sh
test/grdconvert/grdconvert-l2s.sh         test/pswiggle/pswiggle-l2s.sh
test/grdcut/grdcut-l2s.sh                 test/psxy/psxy-l2s.sh
test/grdedit/grdedit-l2s.sh               test/psxyz/psxyz-l2s.sh
test/grdfft/grdfft-l2s.sh                 test/sample1d/sample1d-l2s.sh
test/grdfill/grdfill-l2s.sh               test/seis/pscoupe-l2s.sh
test/grdfilter/grdfilter-l2s.sh           test/seis/psmeca-l2s.sh
test/grdgdal/grdgdal-l2s.sh               test/spectrum1d/spectrum1d-l2s.sh
test/grdgradient/grdgradient-l2s.sh       test/sph2grd/sph2grd-l2s.sh
test/grdhisteq/grdhisteq-l2s.sh           test/sphdistance/sphdistance-l2s.sh
test/grdimage/grdimage-l2s.sh             test/sphinterpolate/sphinterpolate-l2s.sh
test/grdinfo/grdinfo-l2s.sh               test/sphtriangulate/sphtriangulate-l2s.sh
test/grdinterpolate/grdinterpolate-l2s.sh test/subplot/subplot-l2s.sh
test/grdlandmask/grdlandmask-l2s.sh       test/surface/surface-l2s.sh
test/grdmask/grdmask-l2s.sh               test/trend1d/trend1d-l2s.sh
test/grdmath/grdmath-l2s.sh               test/trend2d/trend2d-l2s.sh
test/grdmix/grdmix-l2s.sh                 test/triangulate/triangulate-l2s.sh
test/grdpaste/grdpaste-l2s.sh             test/xyz2grd/xyz2grd-l2s.sh
test/grdproject/grdproject-l2s.sh

I think it makes more sense to put all l2s tests into a separate directory (e.g., test/l2s or test/long2short) instead.

Thoughts?

Ping @joa-quim @rbdavis

@joa-quim
Copy link
Member

joa-quim commented Apr 8, 2024

Hmm, all tests are in their own directory with that module name. Why making it different for the ``*.-l2s.sh` tests? I prefer to have them where they are.

@seisman
Copy link
Member Author

seisman commented Apr 8, 2024

The main reason is, these tests work only when USE_COMMON_LONG_OPTIONS and USE_MODULE_LONG_OPTIONS are defined. When they're not defined, these tests should be excluded. However, the current layout (e.g., having psbasemap-l2s.sh in the psbasemap directory) makes it difficult to exclude these directory. Instead, if all l2s tests are located in the test/long2short directory, we just need to exclude the whole test/long2short directory.

@joa-quim
Copy link
Member

joa-quim commented Apr 8, 2024

OK, but probably what we should do is to make the USE_COMMON_LONG_OPTIONS and USE_MODULE_LONG_OPTIONS the default now. The work is mostly done and does not interfere with the rest. What is needed is to document them (big task).

@joa-quim
Copy link
Member

I we can close this now, I think.

@seisman
Copy link
Member Author

seisman commented Apr 17, 2024

I still prefer to move all l2s tests into a separate test directory, since these tests test the codes in gmt/src/longopt rather than the functionality of individual modules.

@joa-quim
Copy link
Member

In fact, even the fact that the longopt is currently in a bunch of separate headers was discussed at the beginning of this work and the idea was to revise that decision at the end. For example I would rather have each of those .h inserted directly inside the corresponding C code. Anyway, I see no advantage in joining all the l2s tests in a single directory as that makes needlessly harder to find a specific test as comparing to finding it in the directory where all other tests of that module reside.

@seisman
Copy link
Member Author

seisman commented Apr 17, 2024

In fact, even the fact that the longopt is currently in a bunch of separate headers was discussed at the beginning of this work and the idea was to revise that decision at the end. For example I would rather have each of those .h inserted directly inside the corresponding C code.

Makes sense to me. Closing.

@seisman seisman closed this as completed Apr 17, 2024
@joa-quim
Copy link
Member

Perfect thanks, then one of these days I'll start the .h integration move but we'll have to take care to leave room for the missing supplements longoptions.

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

No branches or pull requests

2 participants