lc-gcr test and other improvements - #528
Conversation
There was a problem hiding this comment.
Please don't upload binary files to the repository.
There was a problem hiding this comment.
file deleted
Please take the test file from /perm/marm/sd/DGOV-513_lcgcr/convert/ok/lc-gcr_sttd_grib2.tmpl
joobog
left a comment
joobog
left a comment
There was a problem hiding this comment.
The test needs improvement
|
|
||
| . ./include.ctest.sh | ||
|
|
||
| grib2_sample=$ECCODES_SAMPLES_PATH/lc-gcr_sttd_grib2.tmpl |
There was a problem hiding this comment.
Instead of using a new files, it should be possible to create new LCGCR file from GRIB2.tmpl sample, by setting corresponding keys using grib_set or grib_filter. It's also an important part of the test.
Another advantage of removing the dependency on a test file is that this test no longer needs to be in EXTRA_TESTS. It can be moved to the regular tests.
There was a problem hiding this comment.
I could see similar dedicated test files in the repo like for Destine. I would prefer to keep that new dedicated test file for LC-GCR (and even to add another similar one in the future)
Originally I started from GRIB2.tmpl but to add all details to get fully compliant lc-gcr file (passing grib-check) requires comprehensive grib_filter. Thus I rather created a test file with all that metadata already there, so that the test scripts is rather short and understandable, similar to other test scripts.
| # In applying this licence, ECMWF does not waive the privileges and immunities granted to it by | ||
| # virtue of its status as an intergovernmental organisation nor does it submit to any jurisdiction. | ||
| # | ||
|
|
There was a problem hiding this comment.
Description of the test is missing. Please add and fill the following:
# ---------------------------------------------------------
# This is the test for JIRA issue ECC-XXXX
# < Add issue summary here >
# ---------------------------------------------------------
Please note, you can use ./tests/create_test.sh script to create test templates.
| #if (modelName isnot "unknown") { | ||
| # concept modelVersion(unknown, "modelVersionConcept.[modelName].def", conceptsDir2, conceptsLocalDirAll): no_copy, dump, read_only; | ||
| # alias ls.model = modelName; | ||
| # alias mars.model = modelName; | ||
| #} |
There was a problem hiding this comment.
Why do we need these comments?
There was a problem hiding this comment.
Older solution removed now.
|
|
||
|
|
||
| # Clean up | ||
| #rm -f $temp1 $temp2 $temp3 $tempLog |
There was a problem hiding this comment.
This test is creating temporary data. Please clean up.
| grib_ecc-2254 | ||
| grib_ecc-2264 | ||
| grib_ecc-2265 | ||
| grib_lc-gcr |
There was a problem hiding this comment.
The test is using test data, but is located in basic checks. Tests, which depend on test data, which must be downloaded, must be located in extra tests.
There was a problem hiding this comment.
OK will fix it. Still my test worked even under standard tests..
Maybe amend FAQ for similar cases
https://confluence.ecmwf.int/spaces/ECC/pages/233246465/ecCodes+developer+FAQ+-+General#ecCodesdeveloperFAQGeneral-HowdoIaddanewtest?
2 participants
Description
https://jira.ecmwf.int/browse/ECC-2314
Contributor Declaration
By opening this pull request, I affirm the following: