mb_regress.sh gripes

Started by Josh Kupershmidtover 14 years ago3 messages
#1Josh Kupershmidt
schmiddy@gmail.com
1 attachment(s)

Hi all,

A few gripes about mb_regress.sh:
1. No exit code is specified, so even if there are differences
between results/ and expected/ the script will still return 0.

2. The 'dropdb' command is used to wipe out the "utf8" database
before the run. This generates an error message like:
dropdb: database removal failed: ERROR: database "utf8" does not exist

the first time you run the script. IMO it would be less startling to
just print a NOTICE here.

3. No error checking for whether createdb succeeds.

The attached patch fixes these problems.

Josh

Attachments:

mb_regress_gripes.patchapplication/octet-stream; name=mb_regress_gripes.patchDownload
diff --git a/src/test/mb/mbregress.sh b/src/test/mb/mbregress.sh
new file mode 100644
index 20942e3..c2f27ef
*** a/src/test/mb/mbregress.sh
--- b/src/test/mb/mbregress.sh
*************** if [ ! -d results ];then
*** 14,24 ****
      mkdir results
  fi
  
- dropdb utf8
- createdb -T template0 -l C -E UTF8 utf8
- 
  PSQL="psql -n -e -q"
  tests="euc_jp sjis euc_kr euc_cn euc_tw big5 utf8 mule_internal"
  unset PGCLIENTENCODING
  for i in $tests
  do
--- 14,28 ----
      mkdir results
  fi
  
  PSQL="psql -n -e -q"
  tests="euc_jp sjis euc_kr euc_cn euc_tw big5 utf8 mule_internal"
+ NUM_FAILURES=0
+ 
+ # Mimic the 'dropdb' command, except don't raise an error message if
+ # the database doesn't already exist.
+ echo 'DROP DATABASE IF EXISTS utf8' | $PSQL template1
+ createdb -T template0 -l C -E UTF8 utf8 || exit 1
+ 
  unset PGCLIENTENCODING
  for i in $tests
  do
*************** do
*** 54,60 ****
--- 58,67 ----
  		echo "----------------------"; \
  		echo "" ) >> regression.diffs
  		echo failed
+ 		NUM_FAILURES=$((NUM_FAILURES+1))
  	else
  		echo ok
  	fi
  done
+ 
+ exit $NUM_FAILURES
#2Robert Haas
robertmhaas@gmail.com
In reply to: Josh Kupershmidt (#1)
Re: mb_regress.sh gripes

On Thu, Aug 18, 2011 at 6:19 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote:

A few gripes about mb_regress.sh:
 1. No exit code is specified, so even if there are differences
between results/ and expected/ the script will still return 0.

 2. The 'dropdb' command is used to wipe out the "utf8" database
before the run. This generates an error message like:
 dropdb: database removal failed: ERROR:  database "utf8" does not exist

the first time you run the script. IMO it would be less startling to
just print a NOTICE here.

 3. No error checking for whether createdb succeeds.

The attached patch fixes these problems.

Committed, with some changes. I used the new --if-exists option for
dropdb rather than doing it as you had it here; I assume this may have
been the motivation for that patch. I also just made the exit code 1
no matter how many failures there were. That seems more normal, and I
wasn't altogether certain that $((expr)) is completely portable. I
also set the execute bit on the script.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#3Josh Kupershmidt
schmiddy@gmail.com
In reply to: Robert Haas (#2)
Re: mb_regress.sh gripes

On Thu, Sep 1, 2011 at 9:25 AM, Robert Haas <robertmhaas@gmail.com> wrote:

Committed, with some changes.  I used the new --if-exists option for
dropdb rather than doing it as you had it here; I assume this may have
been the motivation for that patch.

Thanks, and that was indeed one reason I wanted dropdb --if-exists.

 I also just made the exit code 1
no matter how many failures there were.  That seems more normal, and I
wasn't altogether certain that $((expr)) is completely portable.

Yeah, I wasn't sure about that either. The old book "The Unix
Programming Environment" (section "Arithmetic in sh") claims that
using expr or bc is the only way to do such arithmetic, e.g.:
var=`expr $var + 1`

But then [1]http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_04 documents arithmetic expansion as used in the patch. But
anyway, just returning 1 as you've done seems fine.

Josh

[1]: http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_04