Test scripts need to include set -e

[edwardhartnett]

You must have "set -e" at the top of all your test scripts.

Default shell behavior when a command fails is to continue. Even if commands fail, the script can report success.

If you type help set in a bash shell you can see all the options:

-e Exit immediately if a command exits with a non-zero status.
Here is a test script from netcdf-c which illustrates this. Note that we also sometimes use:

-x Print commands and their arguments as they are executed.

This is very helpful for finding out what went wrong. In fact, it's the first step in debugging a failure, so might as well just turn this on for the test.

#!/bin/sh

# This script runs some PnetCDF I/O tests

set -e
echo
echo "Testing file created with PnetCDF is modifiable with netCDF..."
./tst_pnetcdf

echo "Testing file created with PnetCDF works when adding variables..."
./tst_addvar tst_pnetcdf.nc

# We assume a min of at least 2 processors is available
mpiexec -n 2 ./tst_parallel2

[edwardhartnett]

I did a branch with this, and found some more namelist problems, which I have addressed in a separate PR.

However, I also found that set -e seemed to catch some test failures. Also the use of the "skip" in bats seems to cause a non-zero exit code to be returned - the sign of an error. I would have thought that skip would return success, since it a test is intentionally being skipped, but that is not what happens.

After I get my PR with namelist fixes working, I will start adding set -e to the test scripts and see what happens.

[edwardhartnett]

OK, looks like bats has been deprecated: sstephenson/bats#269

There is something called bats-core that replaces it.

What I would suggest instead is that bats be removed from the build and not used at all.

[underwoo]

We have a goal to remove bats as a requirement for testing. However, due to other commitments, we have not been able to take on that task. I have opened an issue (#221) requesting this to be done. -- Seth Underwood S/W Development Process Lead Modeling Systems Group GFDL/NOAA/DOC 201 Forrestal Road Princeton, NJ 08540-6649 (609) 452-5847 Office (304) 376-9002 Cell (609) 987-5063 Fax [email protected]

…

On Tue, Nov 26, 2019 at 11:11 AM Edward Hartnett ***@***.***> wrote: OK, looks like bats has been deprecated: sstephenson/bats#269 <sstephenson/bats#269> There is something called bats-core that replaces it. What I would suggest instead is that bats be removed from the build and not used at all. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#169?email_source=notifications&email_token=AADJUSZWPEZLQP5KP5R4KOLQVVDDTA5CNFSM4IYZJP72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFGRYRA#issuecomment-558701636>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADJUS7VIEPIOZ4FF7GXCNLQVVDDTANCNFSM4IYZJP7Q> .

[edwardhartnett]

I can take it out of the existing code base, starting today. Will this be acceptable?

The path I will take is first to introduce sh scripts that do the tests currently done by bats. During this phase, both bats and shell scripts will work, selectable by configure. At this point, new tests should only be written as bash shell scripts - no more bats tests should be added. You must communicate that to the other programmers on the project. They can use the ones I provide as examples when writing new ones.

When we are confident all functionality has been achieved by the shell script tests, I will take out bats.

Sound good? Right now I am in my coding cave, with plenty of coffee and 3+ feet of snow on the ground. Let's see what we can get done... ;-)

[edwardhartnett]

OK, bats has been removed, and now all scripts call set -e by including test_common.sh. So I will close this issue.