Re: [Quilt-dev] test suite improvements

[Andreas Gruenbacher]

John,

I quickly checked some of your patches. Here are some comments.

On Friday 13 January 2006 12:29, John Vandenberg wrote:
> Hi again,
>
> In order to complete the portability series that I have been working
> on, there are a few improvements that I would like to propose for the
> test harness/suite.
>
> 1. I like to build test cases for bugs, and then layer a bug fix on
> top of that, so the repercussions of the bug fix can be seen.
>
> I would like to take this one step further, and record that the
> recorded output is incorrect.  Instead of prefixing the line of output
> with >, undesirable output could use ? instead.  test cases for bugs
> can then be committed as soon as they are reproducible, found using
> grep, and worked on at a later date.
>
> Does anyone object to having the bug test case being committed prior
> to the bug fix?

This may be useful, I don't know.

> 2. As part of my original attempt to port quilt to msys, I found a
> number of binaries were producing slightly different messages to those
> in the test cases.  The worst offender was patch-2.5.  While it would
> seem a little anal to try to attempt to support such a historical
> release, it works fine and msys is a pretty finicky platform to
> maintain packages for, so I dont expect a new version of patch being
> able to compile on it for quite a while.  Also, I am sure that
> sometime in the future, a binary that quilt uses will decide to emit a
> slightly different error message, and it will break the test cases.
>
> In order to address this, I propose the following changes to the test
> harness:
>
> 2.1 regex support for output lines that commence with >~
> 2.2 failed commands to use the separators !~, =~, != and == instead of |
> and ?

http://zeroj.hda0.net/quilt/patches/test-regex.diff looks good. Please make 
sure that test/run will parse both '>~foo' and '>~ foo' at the beginning of 
lines. Putting a test case in test/*.test that fails on purpose is not a good 
idea.

I don't know if we need http://zeroj.hda0.net/quilt/patches/test-test.diff. In 
any case, this file better shouldn't be called *.test.

I have changed things so that the test suite runs against the source tree 
instead of the installed version, so 
http://zeroj.hda0.net/quilt/patches/test-this-quilt.diff should be obsolete.

> 3. local-quilt-x.diff introduces a 'make check'.  This becomes very
> important as more of the ./configure information becomes necessary to
> accurately test the quilt built in the local workpit.  Is it ok to
> remove test/Makefile?  The only alternative that I can see is to
> replace it with test/Makefile.in.

I didn't find this patch.

> 4. regularly running all of the tests on msys is really slow.  I would
> like to record which tests have run to completion, and pick up the
> testing on the last test case to fail.  I wrote a rough patch to do
> this last year; any suggestions on how this should be done in order to
> be committed?
>
> http://zeroj.hda0.net/quilt/patches/test-make-depend.diff

Can you remove *.ok at the end of a full, successful test suite run? Then the 
test suite can be re-run without a clean in between, and ``make status'' also 
shouldn't be needed. Could you please try to set TESTS in test/Makefile to 
the tests that should be tried (using filter-out) instead of what you are 
doing right now? This allows ``make TESTS=...''. Something like 
``$(filter-out $(patsubst %.ok,%.test,$(wildcard *.ok)),$(TESTS))'' should 
do.

> 5. Also, when one command in a test case fails, the rest are sure to
> follow.  I would like to change the test harness so that it only
> continues past the first error if an option (--ignore-error) is
> enabled.  I think this behaviour should be disabled by default.

Agreed. I'm fine with changing the default behavior.

Other stuff:

http://zeroj.hda0.net/quilt/patches/test-ignore-on-failure.diff
Could you please use -k as in ``make -k'' instead of -q in test/run?
I don't see what the ``$?'' syntax in *.test is good for.

http://zeroj.hda0.net/quilt/patches/patch-2.5.diff has changes like:
-       > 1 out of 1 hunk FAILED -- rejects in file Oberon.txt
+       >~ 1 out of 1 hunk FAILED -- (saving )?rejects (in file Oberon.txt|to 
\/tmp\/.*)

Instead, please fix cleanup_patch_output() as shown in the attached 
fix-cleanup_patch_output.diff.

http://zeroj.hda0.net/quilt/patches/test-msys.diff:
Please use $(filter-out some.test,$TESTS) instead of patsubst. I don't think 
it's great to hardcode "MINGW32" in anything but configure.in, but in this 
case it might be better than the alternatives.

http://zeroj.hda0.net/quilt/patches/msys-prefix-colon.diff
What's this, and why do we need it? No code duplication please; this looks 
like a job for configure if needed at all.
The SET_PATH thing should be obsolete now.
f
http://zeroj.hda0.net/quilt/patches/msys-prefix-colon.diff
I hope we'll find better fixes for those things.

http://zeroj.hda0.net/quilt/patches/bash-2.04.diff
Which tests need bash extensions and don't run with a /bin/sh?

http://zeroj.hda0.net/quilt/patches/quilt-exec.diff
What's that?

Thanks for all your work,
Andreas

fix-cleanup_patch_output.diff
Description: Text Data