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

Fix errors in cice.setup script and add travis tests to catch this sooner #406

Merged
merged 2 commits into from
Feb 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ install:
#-e 'mirror /archive/Model-Data/CICE/ ~/ICEPACK_INPUTDATA; quit'"

script:
# verify cice.setup --case and cice.setup --test don't error then run test suite
- "./cice.setup --case trcase --mach travisCI --env gnu --pes 2x2 -s diag1"
- "./cice.setup --test smoke --testid trtest --mach travisCI --env gnu
--pes 2x2 -s diag1"
- "./cice.setup --suite travis_suite --testid travisCItest
--mach travisCI --env gnu;
cd testsuite.travisCItest &&
Expand Down
13 changes: 6 additions & 7 deletions cice.setup
Original file line number Diff line number Diff line change
Expand Up @@ -389,13 +389,11 @@ else
cat ${tarray} >> $tsfile
else if (-e ${tarray}.ts) then
cat ${tarray}.ts >> $tsfile
else
if (-e ${ICE_SCRIPTS}/tests/${tarray}.ts) then
cat ${ICE_SCRIPTS}/tests/${tarray}.ts >> $tsfile
else
echo "${0}: ERROR, cannot find testsuite file ${tarray}, also checked ${ICE_SCRIPTS}/tests/${tarray}.ts"
exit -1
endif
else if (-e ${ICE_SCRIPTS}/tests/${tarray}.ts) then
cat ${ICE_SCRIPTS}/tests/${tarray}.ts >> $tsfile
else
echo "${0}: ERROR, cannot find testsuite file ${tarray}, also checked ${ICE_SCRIPTS}/tests/${tarray}.ts"
exit -1
Comment on lines +392 to +396
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this just simplifies the logic, right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct. noticed this as I was debugging.

endif
end

Expand All @@ -422,6 +420,7 @@ set dorun = false
set dosubmit = true
if (\$?SUITE_BUILD) then
set dobuild = "\${SUITE_BUILD}"
endif
Copy link
Member

@phil-blain phil-blain Feb 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so from what I understand and tested this forgotten endif was the cause of the

else: endif not found.

error. I am pretty surprised, since this is inside the heredoc, so it is just some text written to a file, still the shell running cice.setup dies with a syntax error... the mysteries of the C shell!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly. this is pretty shocking. The endif is missing in the job.submit script yet that script never complained. This is exactly what I had been testing all along. Then when we run a non-suite with cice.setup, it failed due to the missing endif in the part of the script that was generating the job.submit suite script. I really cannot believe how this bug was manifested in the end.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kind of weird behavior usually means there's some other bug, doesn't it? Is it possible to step through the script to find out why it hit that block of code? Or is this a "compiler" bug?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this weird behavior is due to the peculiarities of the C shell. As far as I know it is not possible to step a shell script...

So yes I think this is could be classified as a bug in the C shell.

if (\$?SUITE_RUN) then
set dorun = "\${SUITE_RUN}"
endif
Expand Down