----- Original Message ----- > Updated Branches: > refs/heads/master bb275c046 -> 664ca3987 > > > TS-1755: add basic logstats tests > [snip] > > ---------------------------------------------------------------------- > CHANGES | 3 +- > lib/ts/Layout.cc | 4 +- > proxy/Makefile.am | 4 + > proxy/tests/logstats.blog | Bin 0 -> 14976 bytes > proxy/tests/logstats.json | 243 ++++++++++++++++++++++++++++++++ > proxy/tests/logstats.summary | 164 +++++++++++++++++++++ > proxy/tests/test_logstats_json | 26 ++++ > proxy/tests/test_logstats_summary | 25 ++++ > 8 files changed, 466 insertions(+), 3 deletions(-) > ---------------------------------------------------------------------- > > http://git-wip-us.apache.org/repos/asf/trafficserver/blob/664ca398/proxy/tests/test_logstats_json > ---------------------------------------------------------------------- > diff --git a/proxy/tests/test_logstats_json > b/proxy/tests/test_logstats_json > new file mode 100755 > index 0000000..58de9f7 > --- /dev/null > +++ b/proxy/tests/test_logstats_json > @@ -0,0 +1,26 @@ > +#! /usr/bin/env bash
This is bash. But.. [snip] > + > +set -e # exit on error > + > +TMPFILE=`mktemp -t logstats.XXXXXX` || exit 1 You're using ``, which is frowned upon sh syntax. (makes for poor readability according to Bash gods. Use $( command ) Also: set -e not catches a faulty return from mktemp, no use to use || exit 1. Please only use UPPERCASE for bash variables, not your OWN. Is TMPFILE overridable from the outside? No, then THERE'S NO NEED FOR CAPS. > + > +# Note that the JSON has a timestamp in it that we have to filter > out ... > +./traffic_logstats --log_file "$srcdir/tests/logstats.blog" --json | > grep -v timestamp > $TMPFILE > +diff -q $TMPFILE "$srcdir/tests/logstats.json" > +rm -f $TMPFILE please use more quotes (around $TMPFILE ), and perhaps use rm -f -- so no one can accidentaly pass -r $TMPFILE > http://git-wip-us.apache.org/repos/asf/trafficserver/blob/664ca398/proxy/tests/test_logstats_summary > ---------------------------------------------------------------------- > diff --git a/proxy/tests/test_logstats_summary > b/proxy/tests/test_logstats_summary > new file mode 100755 > index 0000000..e41a33e > --- /dev/null > +++ b/proxy/tests/test_logstats_summary > @@ -0,0 +1,25 @@ > +#! /usr/bin/env sh This is "sh". Yay consistency. > +# Licensed to the Apache Software Foundation (ASF) under one > +# or more contributor license agreements. See the NOTICE file > +# distributed with this work for additional information > +# regarding copyright ownership. The ASF licenses this file > +# to you under the Apache License, Version 2.0 (the > +# "License"); you may not use this file except in compliance > +# with the License. You may obtain a copy of the License at > +# > +# http://www.apache.org/licenses/LICENSE-2.0 > +# > +# Unless required by applicable law or agreed to in writing, > software > +# distributed under the License is distributed on an "AS IS" BASIS, > +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > implied. > +# See the License for the specific language governing permissions > and > +# limitations under the License. > + > +set -e # exit on error > + > +TMPFILE=`mktemp -t logstats.XXXXXX` || exit 1 > + > +./traffic_logstats --log_file "$srcdir/tests/logstats.blog" > --summary > $TMPFILE > +diff -q $TMPFILE "$srcdir/tests/logstats.summary" > +rm -f $TMPFILE same as obove.. In his day job Igor writes fierce reviews of horrific shell scripts, which is why people have stopped committing stuff altogether.. Igor is NOT paranoid. -- Igor Galić Tel: +43 (0) 664 886 22 883 Mail: [email protected] URL: http://brainsware.org/ GPG: 6880 4155 74BD FD7C B515 2EA5 4B1D 9E08 A097 C9AE
