[develop]: Use yaml format for specifying experiment parameters #778
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| name: Python functional tests | ||
| on: | ||
| push: | ||
| paths: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| name: Python unittests | ||
| on: [push, pull_request, workflow_dispatch] | ||
| jobs: | ||
|
|
||
| python_unittests: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -405,30 +405,10 @@ function get_WE2Etest_names_subdirs_descs() { | |
| var_name_at \ | ||
| vars_to_extract | ||
|
|
||
| local dta \ | ||
| nxny \ | ||
| dta_r \ | ||
| nxny_r | ||
| # | ||
| #----------------------------------------------------------------------- | ||
| # | ||
|
|
@@ -591,7 +571,11 @@ information on all WE2E tests: | |
| # statement will be false. | ||
| # | ||
| if [ -z "${generate_csv_file}" ]; then | ||
| if [ -f "${subdir_fp}/*.yaml" ]; then | ||
| mod_time_subdir=$( stat --format=%Y "${subdir_fp}"/*.yaml | sort -n | tail -1 ) | ||
| else | ||
| mod_time_subdir="0" | ||
| fi | ||
| if [ "${mod_time_subdir}" -gt "${mod_time_csv}" ]; then | ||
| generate_csv_file="TRUE" | ||
| print_info_msg " | ||
|
|
@@ -633,7 +617,7 @@ to generate a new CSV file: | |
| # "config.${test_name}.sh", in which case it will be equal to ${test_name}. | ||
| # Otherwise, it will be a null string. | ||
| # | ||
| regex_search="^config\.(.*)\.yaml$" | ||
| test_name_or_null=$( printf "%s\n" "${crnt_item}" | \ | ||
| sed -n -r -e "s/${regex_search}/\1/p" ) | ||
| # | ||
|
|
@@ -887,7 +871,7 @@ name (test_name) is not 1: | |
| test_name = \"${test_name}\" | ||
| num_occurrences = ${num_occurrences} | ||
| These configuration files all have the name | ||
| \"config.${test_name}.yaml\" | ||
| and are located in the following category subdirectories under | ||
| test_configs_basedir: | ||
| subdirs = ( $( printf "\"%s\" " "${subdirs[@]}" )) | ||
|
|
@@ -995,80 +979,11 @@ configuration files of the primary WE2E tests... | |
| # leading spaces, the hash symbol, and possibly another space and append | ||
| # what remains to the local variable test_desc. | ||
| # | ||
gsketefian marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| config_fn="config.${test_name}.yaml" | ||
|
There was a problem hiding this comment. I generated a csv file containing test info and after opening it in google sheets, I found a couple of issues. At least in some places in the test descriptions, newlines are dropped and single quotes (apostrophes) are replaced with double quotes. For example, for the test but in the csv file, it becomes: So the newline/indentation is ignored and the apostrophes in App's User's Guide are changed to double quotes. Any chance we can fix that up? There was a problem hiding this comment. Just randomly checking other descriptions in the google sheet, I see a similar issue for the test There was a problem hiding this comment. @gsketefian Yes, I replace single quotes with double quotes, not only in we2e test description but also in comments in crontab contents. Usually you get empty crontab content but Venita's automated testing setup had crontab contents with comments that had single quotes. Please take a look at PR #816 that describs the issue and how I solved it. I didn't find a better solution than that at the time ... There was a problem hiding this comment. @danielabdi-noaa In this case of the test descriptions, it doesn't seem to make sense to do that replacement. What you really want to do is take the description and put it into the csv file exactly as is. Replacing apostrophes with double quotes would be very confusing to someone reading the test description in the csv. Since this is separate from the treatment of crontab lines, is it possible to not do the replacement when dealing with these descriptions? There must be a way to just take a literal string from the yaml file (even one that includes single quotes and newlines) and place it exactly as-is into the csv. There was a problem hiding this comment. @gsketefian I have made the we2e test description specifically (not For the record though, I liked the way it was where the test description is one long line, because, in the excel sheet it will be wrapped around cell width and looks natural. While with this new fix that keeps the format first used in the config file, widening the cell has no effect. Also now it is not consistent with the test description in There was a problem hiding this comment. @danielabdi-noaa Sorry I wasn't clear about the newlines (or suggested the wrong thing). Yes, I like it to be one long string too (that's how it is now) because that's what works best in a spreadsheet, except when there is a blank line, which signifies a new paragraph or newline. Currently, if you have this in the description in the test config file (in bash): it will show up like this in a cell in the spreadsheet: So the lines without a blank line in between get concatenated into one long line, but the blank lines cause there to be a newline to be inserted. Also, the indentations for the lines |
||
| config_fp="${test_configs_basedir}/$subdir/$config_fn" | ||
| test_desc="$(config_to_shell_str $config_fp -k "metadata")" | ||
| test_desc="${test_desc:26}" | ||
| test_desc="${test_desc::${#test_desc}-1}" | ||
| # | ||
| # Finally, save the description of the current test as the next element | ||
| # of the array prim_test_descs. | ||
|
|
@@ -1079,10 +994,11 @@ ${test_desc}${stripped_line} " | |
| # variables specified in "vars_to_extract". Then save the value in the | ||
| # arrays specified by "prim_array_names_vars_to_extract". | ||
| # | ||
| config_content=$(config_to_shell_str $config_fp) | ||
| for (( k=0; k<=$((num_vars_to_extract-1)); k++ )); do | ||
|
|
||
| var_name="${vars_to_extract[$k]}" | ||
| cmd=$( grep "^[ ]*${var_name}=" <<< "${config_content}" ) | ||
| eval $cmd | ||
|
|
||
| if [ -z "${!var_name+x}" ]; then | ||
|
|
@@ -1689,49 +1605,8 @@ containing information on all WE2E tests: | |
| fi | ||
|
|
||
| if [ ! -z "${outvarname_test_descs}" ]; then | ||
|
There was a problem hiding this comment. I guess removal of this substitution is what's causing the apostrophes (single quotes) to become double quotes. There was a problem hiding this comment. The replacement is done whenever you produce a shell string equivalent of other formats. Bash does not like single quotes since they have been causing me problems in unexpected places (two places I already mentioned but there were of others too), so I decided to get rid of them in the config parser: # replace some problematic chars
v1 = v1.replace("'", '"')
v1 = v1.replace("\n", " ")
# end problematic
shell_str += f"{k}='{v1}'\n"Also I replace newlines with space for the same reason, without which I would not be able to store the test description in There was a problem hiding this comment. Here is an example # [metadata]
description='This test checks the capability of the workflow to have the user specify a new grid (as opposed to one of the predefined ones in the workflow) of GFDLgrid type. Note that this test sets the workflow variable GFDLgrid_USE_NUM_CELLS_IN_FILENAMES to "TRUE" (which is its default value); see the UFS SRW User"s Guide for a description of this variable. The difference between this test and the one named custom_GFDLgrid__GFDLgrid_USE_NUM_CELLS_IN_FILENAMES_eq_TRUE is that this one uses almost no stretching by setting the workflow variable GFDLgrid_STRETCH_FAC very close to 1. Setting it exactly to 1 used to cause the workflow to fail because it caused the GFDL grid generator to assume a global grid. This bug should be rechecked, e.g. by setting GFDLgrid_STRETCH_FAC to exactly 1 below. If the grid generation succeeds, then this test can be removed.'
version='1.0'
# [user]
RUN_ENVIR='community'
MACHINE='HERA'
MACHINE_FILE='/scratch2/BMC/gsd-hpcs/Daniel.Abdi/ufs-srweather-app/regional_workflow/ush/machine/hera.sh'
ACCOUNT='zrtrr'
# [platform]
WORKFLOW_MANAGER='rocoto'
NCORES_PER_NODE='40'That multiline description would cause problems if it had "single quotes" or "new line" characters. There was a problem hiding this comment. Ok, I see. I didn't know the test description goes in |
||
| test_descs_str="( "$( printf "'%s' " "${test_descs[@]}" )")" | ||
| eval ${output_varname_test_descs}="${test_descs_str}" | ||
| fi | ||
| # | ||
| #----------------------------------------------------------------------- | ||
|
|
||
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.
Does this mean we have a button to run unittests on GitHub runners any time we want from any branch?
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.
That is the idea. I did check that I can manually re-run a workflow on my fork, but things could be different once it is merged here.