By the way, shouldn't the rule hs-tests-% also delete the file htest.tix ?
Just like the rule hs-tests does ?


On Wed, Jul 10, 2013 at 10:16 AM, Michele Tartara <[email protected]>wrote:

> On Tue, Jul 9, 2013 at 6:58 PM, Jose A. Lopes <[email protected]>wrote:
>
>> From: "Jose A. Lopes" <[email protected]>
>>
>>
> The title of the patch should be at most 60 characters long. This one is
> 61 :-( and has to be shortened.
>
>
>> * add Makefile.am target hs-tests-% for Haskell single-execution
>
> * add Makefile.am target hs-shell-% for shelltests single-execution
>>
>
> The commit message should be a proper sentence, complete with punctuation.
> A list of items usually means that the patch is too big and should have
> been split in two patches part of a patch set.
> In this case, the patch is short enough that can be sent as a single one,
> but at the very least, add a full stop at the end of the second line.
>
>
>>
>> Signed-off-by: Jose A. Lopes <[email protected]>
>> ---
>>  Makefile.am             | 19 ++++++++++++++++++-
>>  test/hs/offline-test.sh | 13 +++++++++----
>>  2 files changed, 27 insertions(+), 5 deletions(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index dd9dbd2..18c62c0 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -20,6 +20,9 @@ strip_hsroot = $(patsubst src/%,%,$(patsubst
>> test/hs/%,%,$(1)))
>>  # Use bash in order to be able to use pipefail
>>  SHELL=/bin/bash
>>
>> +# Enable colors in shelltest
>> +SHELLTESTARGS = "-c"
>> +
>>  ACLOCAL_AMFLAGS = -I autotools
>>  BUILD_BASH_COMPLETION = $(top_srcdir)/autotools/build-bash-completion
>>  RUN_IN_TEMPDIR = $(top_srcdir)/autotools/run-in-tempdir
>> @@ -1827,15 +1830,29 @@ check-local: check-dirs $(GENERATED_FILES)
>>         done; \
>>         test -z "$$error"
>>
>> +.PHONY: hs-tests-%
>> +hs-tests-%: test/hs/htest
>>
>
> Given that the rule is for launching a single test, what about naming it
> hs-test-% instead of hs-tests-% ?
>
>
>> +       @test/hs/htest --list-tests | grep $*
>>
>
> The names of the tests are printed during their execution anyway. Why
> printing it again with this line?
>
>
>> +       test/hs/htest -t $*
>> +
>>  .PHONY: hs-tests
>>  hs-tests: test/hs/htest | $(BUILT_PYTHON_SOURCES)
>>         @rm -f htest.tix
>>         ./test/hs/htest
>>
>> +.PHONY: hs-shell-%
>> +hs-shell-%: test/hs/hpc-htools test/hs/hpc-mon-collector
>> $(HS_BUILT_TEST_HELPERS)
>> +       @rm -f hpc-htools.tix hpc-mon-collector.tix
>> +       HBINARY="./test/hs/hpc-htools" \
>> +       SHELLTESTARGS=$(SHELLTESTARGS) \
>> +       ./test/hs/offline-test.sh $*
>> +
>>  .PHONY: hs-shell
>>  hs-shell: test/hs/hpc-htools test/hs/hpc-mon-collector
>> $(HS_BUILT_TEST_HELPERS)
>>         @rm -f hpc-htools.tix hpc-mon-collector.tix
>> -       HBINARY="./test/hs/hpc-htools" ./test/hs/offline-test.sh
>> +       HBINARY="./test/hs/hpc-htools" \
>> +       SHELLTESTARGS=$(SHELLTESTARGS) \
>> +       ./test/hs/offline-test.sh
>>
>>  .PHONY: hs-check
>>  hs-check: hs-tests hs-shell
>> diff --git a/test/hs/offline-test.sh b/test/hs/offline-test.sh
>> index 07f002e..b3b5427 100755
>> --- a/test/hs/offline-test.sh
>> +++ b/test/hs/offline-test.sh
>> @@ -20,6 +20,14 @@
>>  # This is an offline testing script for most/all of the htools
>>  # programs, checking basic command line functionality.
>>
>> +# Optional argument that specifies the test files to run. If not
>> +# specified, then all tests are run.
>> +#
>> +# For example, a value of 'balancing' runs the file
>> +# 'shelltests/htools-balancing.test'.  Multiple files can be specified
>> +# using shell notation, for example, '{balancing,basic}'.
>> +TESTS=${1:-*}
>> +
>>  set -e
>>  set -o pipefail
>>
>> @@ -97,7 +105,4 @@ echo OK
>>  echo Running shelltest...
>>
>>  shelltest $SHELLTESTARGS \
>> -  ${TOP_SRCDIR:-.}/test/hs/shelltests/htools-*.test \
>> -  -- --hide-successes
>> -
>> -echo All OK
>> +  ${TOP_SRCDIR:-.}/test/hs/shelltests/htools-$TESTS.test
>> \ No newline at end of file
>> --
>> 1.8.3
>>
>>
> Also, you should update the doc/devnotes.rst file, in the "Running
> individual tests" section, specifying how to run individual tests after
> your modification.
>
> Thanks,
> Michele
>
>

Reply via email to