Re: Automake testsuite misuses DejaGnu [PATCH v0]
Jim Meyering wrote:
[...]
Even a sample fix for one of the currently-failing tests would be helpful.
This is the first draft; this patch breaks 1.6.1 because versions of
DejaGnu prior to 1.6.3 require srcdir to point exactly to the testsuite,
while 1.6.3 allows the testsuite to be in ${srcdir}/testsuite.
8<--
diff -urN -x '*~' automake-1.16.3-original/t/check12.sh
automake-1.16.3/t/check12.sh
--- automake-1.16.3-original/t/check12.sh 2020-11-18 19:21:03.0
-0600
+++ automake-1.16.3/t/check12.sh2021-06-29 01:47:21.669276386 -0500
@@ -60,8 +60,8 @@
DEJATOOL = hammer spanner
AM_RUNTESTFLAGS = HAMMER=$(srcdir)/hammer SPANNER=$(srcdir)/spanner
EXTRA_DIST += $(DEJATOOL)
-EXTRA_DIST += hammer.test/hammer.exp
-EXTRA_DIST += spanner.test/spanner.exp
+EXTRA_DIST += testsuite/hammer.test/hammer.exp
+EXTRA_DIST += testsuite/spanner.test/spanner.exp
END
cat > hammer << 'END'
@@ -77,9 +77,10 @@
END
chmod +x hammer spanner
-mkdir hammer.test spanner.test
+mkdir testsuite
+mkdir testsuite/hammer.test testsuite/spanner.test
-cat > hammer.test/hammer.exp << 'END'
+cat > testsuite/hammer.test/hammer.exp << 'END'
set test test_hammer
spawn $HAMMER
expect {
@@ -88,7 +89,7 @@
}
END
-cat > spanner.test/spanner.exp << 'END'
+cat > testsuite/spanner.test/spanner.exp << 'END'
set test test_spanner
spawn $SPANNER
expect {
diff -urN -x '*~' automake-1.16.3-original/t/dejagnu3.sh
automake-1.16.3/t/dejagnu3.sh
--- automake-1.16.3-original/t/dejagnu3.sh 2020-11-18 19:21:03.0
-0600
+++ automake-1.16.3/t/dejagnu3.sh 2021-06-29 01:19:19.161147525 -0500
@@ -34,12 +34,13 @@
AUTOMAKE_OPTIONS = dejagnu
DEJATOOL = hammer
AM_RUNTESTFLAGS = HAMMER=$(srcdir)/hammer
-EXTRA_DIST = hammer hammer.test/hammer.exp
+EXTRA_DIST = hammer testsuite/hammer.test/hammer.exp
END
-mkdir hammer.test
+mkdir testsuite
+mkdir testsuite/hammer.test
-cat > hammer.test/hammer.exp << 'END'
+cat > testsuite/hammer.test/hammer.exp << 'END'
set test test
spawn $HAMMER
expect {
diff -urN -x '*~' automake-1.16.3-original/t/dejagnu4.sh
automake-1.16.3/t/dejagnu4.sh
--- automake-1.16.3-original/t/dejagnu4.sh 2020-11-18 19:21:03.0
-0600
+++ automake-1.16.3/t/dejagnu4.sh 2021-06-29 01:25:08.309780437 -0500
@@ -49,13 +49,14 @@
AM_RUNTESTFLAGS = HAMMER=$(srcdir)/hammer SPANNER=$(srcdir)/spanner
-EXTRA_DIST = hammer hammer.test/hammer.exp
-EXTRA_DIST += spanner spanner.test/spanner.exp
+EXTRA_DIST = hammer testsuite/hammer.test/hammer.exp
+EXTRA_DIST += spanner testsuite/spanner.test/spanner.exp
END
-mkdir hammer.test spanner.test
+mkdir testsuite
+mkdir testsuite/hammer.test testsuite/spanner.test
-cat > hammer.test/hammer.exp << 'END'
+cat > testsuite/hammer.test/hammer.exp << 'END'
set test test
spawn $HAMMER
expect {
@@ -64,7 +65,7 @@
}
END
-cat > spanner.test/spanner.exp << 'END'
+cat > testsuite/spanner.test/spanner.exp << 'END'
set test test
spawn $SPANNER
expect {
diff -urN -x '*~' automake-1.16.3-original/t/dejagnu5.sh
automake-1.16.3/t/dejagnu5.sh
--- automake-1.16.3-original/t/dejagnu5.sh 2020-11-18 19:21:03.0
-0600
+++ automake-1.16.3/t/dejagnu5.sh 2021-06-29 01:26:36.511645792 -0500
@@ -34,12 +34,13 @@
cat > Makefile.am << END
AUTOMAKE_OPTIONS = dejagnu
-EXTRA_DIST = $package $package.test/$package.exp
+EXTRA_DIST = $package testsuite/$package.test/$package.exp
AM_RUNTESTFLAGS = PACKAGE=\$(srcdir)/$package
END
-mkdir $package.test
-cat > $package.test/$package.exp << 'END'
+mkdir testsuite
+mkdir testsuite/$package.test
+cat > testsuite/$package.test/$package.exp << 'END'
set test "a_dejagnu_test"
spawn $PACKAGE
expect {
diff -urN -x '*~' automake-1.16.3-original/t/dejagnu6.sh
automake-1.16.3/t/dejagnu6.sh
--- automake-1.16.3-original/t/dejagnu6.sh 2020-11-18 19:21:03.0
-0600
+++ automake-1.16.3/t/dejagnu6.sh 2021-06-29 01:28:07.151396859 -0500
@@ -35,8 +35,9 @@
AM_RUNTESTFLAGS = FAILDEJA=$(srcdir)/faildeja
END
-mkdir faildeja.test
-cat > faildeja.test/faildeja.exp << 'END'
+mkdir testsuite
+mkdir testsuite/faildeja.test
+cat > testsuite/faildeja.test/faildeja.exp << 'END'
set test failing_deja_test
spawn $FAILDEJA
expect {
diff -urN -x '*~' automake-1.16.3-original/t/dejagnu7.sh
automake-1.16.3/t/dejagnu7.sh
--- automake-1.16.3-original/t/dejagnu7.sh 2020-11-18 19:21:03.0
-0600
+++ automake-1.16.3/t/dejagnu7.sh 2021-06-29 01:29:38.877097021 -0500
@@ -39,8 +39,9 @@
AM_RUNTESTFLAGS = --status FAILTCL=$(srcdir)/failtcl
END
-mkdir failtcl.test
-cat > failtcl.test/failtcl.exp << 'END'
+mkdir testsuite
+mkdir testsuite/failtcl.test
+cat > testsuite/failtcl.test/failtcl.exp << 'END'
set test test
spawn $FAILTCL
expect {
diff -urN -x '*~' automake-1.16.3-original/t/dejagnu-absolute-builddir.sh
automake-1.16.3/t/dejagnu-absolute-builddir.sh
--- automake-1.16.3-original/t/dejagnu-absolute-builddir.sh 2020-11-18
19:21:03.0 -0600
+++ automake-1.16.3/t/dejagnu-absolute-builddir.sh 2021-06-29
Re: Automake testsuite misuses DejaGnu
Daniel Herring wrote: It seems fragile for DejaGnu to probe for a testsuite directory and change its behavior as you describe. For example, I could have a project without the testsuite dir, invoke the tester, and have it find and run some unrelated files in the parent directory. Unexpected behavior (chaos) may ensue. This already happens and this is the behavior that is deprecated and even more fragile. Without a testsuite/ directory, DejaGnu will end up searching the tree for *.exp files and running them all. Eventually, if $srcdir neither is nor contains "testsuite", DejaGnu will throw an error and abort. The testsuite/ directory is a long-documented requirement. Is there an explicit command-line argument that could be added to the Automake invocation? Not easily; the probing is done specifically to allow for two different ways of using DejaGnu: using recursive Makefiles that invoke DejaGnu with the testsuite/ directory current, and using non-recursive Makefiles, which with Automake will invoke DejaGnu with the top-level directory, presumably containing the "testsuite" directory. Both of these cases must be supported: the toolchain packages use the former and Automake's basic DejaGnu support will use the latter if a non-recursive layout is desired. Both of these use the same command line argument --srcdir and site.exp variable srcdir; the difference is that srcdir has acquired two different meanings. -- Jacob
Re: Automake testsuite misuses DejaGnu
Hi Jacob,
It seems fragile for DejaGnu to probe for a testsuite directory and change
its behavior as you describe. For example, I could have a project without
the testsuite dir, invoke the tester, and have it find and run some
unrelated files in the parent directory. Unexpected behavior (chaos) may
ensue.
Is there an explicit command-line argument that could be added to the
Automake invocation?
-- Daniel
On Mon, 12 Jul 2021, Jacob Bachmeyer wrote:
Karl Berry wrote:
DejaGnu has always required a DejaGnu testsuite to be rooted at a
"testsuite" directory
If something was stated in the documentation, but not enforced by the
code, hardly surprising that "non-conformance" is widespread.
It is not widespread -- all of the toolchain packages correctly place their
testsuites in testsuite/ directories. As far as I know, the Automake tests
are the only outlier.
Anyway, it seems like an unfriendly requirement for users. And even more
to incompatibly enforce something now that has not been enforced for
previous decades. Why? (Just wondering.) -k
Previous versions of DejaGnu did not properly handle non-recursive make with
Automake-produced makefiles. Beginning with 1.6.3, the testsuite is allowed
to be in ${srcdir}/testsuite instead of ${srcdir} exactly. Enforcing the
long-documented (and mostly followed) requirement that there be a directory
named "testsuite" containing the testsuite allows DejaGnu to resolve the
ambiguity and determine if it has been invoked at package top-level or in the
testsuite/ directory directly.
Even in 1.6.3, there was intent to continue to allow the broken cases to work
with a warning, but I made the conditional for that case too narrow (oops!)
and some of the Automake test cases fail as a result. Fixing this now is
appropriate because no one is going to see the future deprecation warnings
due to the way Automake tests are run.
-- Jacob
Re: Automake testsuite misuses DejaGnu
Karl Berry wrote:
DejaGnu has always required a DejaGnu testsuite to be rooted at a
"testsuite" directory
If something was stated in the documentation, but not enforced by the
code, hardly surprising that "non-conformance" is widespread.
It is not widespread -- all of the toolchain packages correctly place
their testsuites in testsuite/ directories. As far as I know, the
Automake tests are the only outlier.
Anyway, it seems like an unfriendly requirement for users. And even more
to incompatibly enforce something now that has not been enforced for
previous decades. Why? (Just wondering.) -k
Previous versions of DejaGnu did not properly handle non-recursive make
with Automake-produced makefiles. Beginning with 1.6.3, the testsuite
is allowed to be in ${srcdir}/testsuite instead of ${srcdir} exactly.
Enforcing the long-documented (and mostly followed) requirement that
there be a directory named "testsuite" containing the testsuite allows
DejaGnu to resolve the ambiguity and determine if it has been invoked at
package top-level or in the testsuite/ directory directly.
Even in 1.6.3, there was intent to continue to allow the broken cases to
work with a warning, but I made the conditional for that case too narrow
(oops!) and some of the Automake test cases fail as a result. Fixing
this now is appropriate because no one is going to see the future
deprecation warnings due to the way Automake tests are run.
-- Jacob
Re: Automake testsuite misuses DejaGnu
Jim Meyering wrote:
On Sun, Jul 11, 2021 at 9:03 PM Jacob Bachmeyer wrote:
[...]
The affected tests are: check12, dejagnu3, dejagnu4, dejagnu5,
dejagnu6, dejagnu7, dejagnu-absolute-builddir, dejagnu-relative-srcdir,
dejgnu-siteexp-extend, dejagnu-siteexp-useredit.
[...]
Thank you for the analysis and heads-up.
I see that Fedora 34 currently has only dejagnu-1.6.1.
If this is something you can help with now, I can certainly wait a few days.
Even a sample fix for one of the currently-failing tests would be helpful.
That is part of the problem: I have a patch, but applying it will cause
the tests to fail with DejaGnu 1.6.1. Older versions of DejaGnu require
$srcdir to be exactly the root of the testsuite, while 1.6.3 accepts a
testsuite in $srcdir or ${srcdir}/testsuite; the latter is needed to
allow Automake to invoke DejaGnu from the top-level in the tree.
I expect to have time to try a recursive make solution later tonight or
tomorrow. Do I understand correctly that I will need to add "SUBDIRS =
testsuite" to the top-level TEST_CASE/Makefile.am in the test case and
move the "AUTOMAKE_OPTIONS = dejagnu" and "DEJATOOL" definitions to
TEST_CASE/testsuite/Makefile.am to get Automake to invoke DejaGnu in the
testsuite subdirectory instead of top-level?
-- Jacob
Re: Automake testsuite misuses DejaGnu
DejaGnu has always required a DejaGnu testsuite to be rooted at a "testsuite" directory If something was stated in the documentation, but not enforced by the code, hardly surprising that "non-conformance" is widespread. Anyway, it seems like an unfriendly requirement for users. And even more to incompatibly enforce something now that has not been enforced for previous decades. Why? (Just wondering.) -k
Re: Automake testsuite misuses DejaGnu
On Sun, Jul 11, 2021 at 9:03 PM Jacob Bachmeyer wrote: > I was planning to find a solution with a complete patch before > mentioning this, but since a release is imminent I will just state the > problem: several tests in the Automake testsuite misuse DejaGnu and > fail with the 1.6.3 DejaGnu release as a result. > > DejaGnu has always required a DejaGnu testsuite to be rooted at a > "testsuite" directory and this has long been documented in the manual. > However, prior to 1.6.3, DejaGnu did not actually depend on this > requirement being met. Changes during the development process to > properly support non-recursive Automake makefiles required relying on > this requirement to resolve the ambiguity between recursive and > non-recursive usage. Several tests in the Automake testsuite do not > meet this requirement and fail if run with DejaGnu 1.6.3. > > The simple change of updating the tests to use a testsuite/ directory > causes the tests to fail with older versions of DejaGnu, due to lack of > support for non-recursive "make check" in those versions. I have not > yet tried a patch that also switches the tests to use recursive make, > but I believe that is probably the only way for the tests to pass with > old and new DejaGnu. > > Note that, according to the original author, Rob Savoye, DejaGnu has > always been intended to require that testsuites be rooted at a > "testsuite" directory and the behavior that Automake's test cases rely > on was never supported. > > The affected tests are: check12, dejagnu3, dejagnu4, dejagnu5, > dejagnu6, dejagnu7, dejagnu-absolute-builddir, dejagnu-relative-srcdir, > dejgnu-siteexp-extend, dejagnu-siteexp-useredit. > > Note that these tests do not all fail with the 1.6.3 release, but will > all fail with some future release when the undocumented support for a > testsuite not rooted at "testsuite" will eventually be removed. Thank you for the analysis and heads-up. I see that Fedora 34 currently has only dejagnu-1.6.1. If this is something you can help with now, I can certainly wait a few days. Even a sample fix for one of the currently-failing tests would be helpful.
