Opened 6 years ago

Closed 6 years ago

#3243 closed enhancement (fixed)

[kean] The serial unit tests execution is not possible for automake 1.13 and later

Reported by: marcin Owned by: kean
Priority: medium Milestone: Sprint-20131015
Component: build system Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Core Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Starting from automake 1.13 the test suites are executed in the parallel mode. Along with this, the unit tests output is not logged to the console, but rather to file. This is very inconvenient for the following reasons:

  • I have to go to individual logs to see what went wrong if my unit tests fail. Note that we have a lot of test suites in bind10
  • I have no output from the tests execution if they all pass. I rather have a summary.

Here is the sample output from the successfully executed test suite:

make[9]: Leaving directory `/home/b10builder/builder/work/BIND10/20131114133526-FedoraLinux-x86_64-GCC/build/src/lib/util/threads/tests'
============================================================================
Testsuite summary for bind10 20130529
============================================================================
# TOTAL: 1
# PASS:  1
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================

It tells me nothing about the test cases being run. This may cause additional problems with diagnosing problems when unit tests are executed on the build farm - browsing multiple logs may be tricky on the build farm.

It seems that the configure.ac has already got a hack for this:

# serial-tests is not available in automake version before 1.13. In
# automake 1.13 and higher, AM_PROG_INSTALL is undefined, so we'll check
# that and conditionally use serial-tests.
AM_INIT_AUTOMAKE(
	[foreign]
	m4_ifndef([AM_PROG_INSTALL], [serial-tests])
)

which relies on the presence or absence of the macro to check the current version of the automake. It seems that the absence of the macro doesn't give a reliable indication of the automake version. From what I have read, the macros have been moved back and forth between versions of automake. Also, I don't know the easy and portable way to check the version of the automake from the configure.ac.

My suggestion is that we add an extra parameter to configure.ac (say --enable-serial-tests) to turn the serial test mode. The behaviour will be serial-mode for automake prior to 1.13 and parallel mode after 1.13.

Subtickets

Change History (16)

comment:1 Changed 6 years ago by shane

  • Milestone changed from New Tasks to Sprint-20131015
  • Summary changed from The serial unit tests execution is not possible for automake 1.13 and later to [kean] The serial unit tests execution is not possible for automake 1.13 and later

comment:2 Changed 6 years ago by kean

  • Owner changed from UnAssigned to kean
  • Status changed from new to accepted

comment:3 Changed 6 years ago by kean

  • Owner changed from kean to UnAssigned
  • Status changed from accepted to reviewing

Sadly we can't use --enable-serial-tests as an option to configure because that is a post-generation option, and we need to specify the automake options during autoreconf time. Checking for AM_PROG_INSTALL is also naive as that macro seems to come and go. So instead, I have created a bootstrap.sh script that drives generating the configure script. Part of its work is to check the version of automake being used and to produce a macro file that is used to set the automake options. By default bootstrap.sh will use the parallel test harness. If you want to use the serial test harness for use on the build farm so you only have a single log file to scrape, invoke bootstrap.sh with the single argument "serial". Updated the guide accordingly as well.

comment:4 Changed 6 years ago by muks

  • Owner changed from UnAssigned to kean

I don't like introduction of the bootstrap script just for this serial harness feature. But this my personal preference. I'm fine with it if other developers (inc. DHCP team) agree this is necessary.

Please can you ask on the bind10-dev@ list what others think?

comment:5 Changed 6 years ago by kean

I think the bootstrap script is useful beyond just this feature. A lot, if not most, of open source programs out there that use autoconf have such a script, because getting the exact right set of options and versions can be tricky. This script insulates us from such trickiness as we can just change the script, rather than having to change individual auto* options in the docs. Its why most other system include just such a script.

comment:6 follow-ups: Changed 6 years ago by marcin

Can we just workaround the serial-tests problem by checking the automake version using system command ?

+m4_define([serial_tests], [
+    m4_esyscmd([automake --version |
+                head -1 |
+                awk '{split ($NF,a,"."); if (a[1] == 1 && a[2] >= 12) { print "serial-tests" }}'
+    ])
+])
+AM_INIT_AUTOMAKE(foreign serial_tests)

I have taken it from here:
https://www.redhat.com/archives/libguestfs/2013-February/msg00102.html

comment:7 in reply to: ↑ 6 Changed 6 years ago by muks

Replying to marcin:

Can we just workaround the serial-tests problem by checking the automake version using system command ?

+m4_define([serial_tests], [
+    m4_esyscmd([automake --version |
+                head -1 |
+                awk '{split ($NF,a,"."); if (a[1] == 1 && a[2] >= 12) { print "serial-tests" }}'
+    ])
+])
+AM_INIT_AUTOMAKE(foreign serial_tests)

I have taken it from here:
https://www.redhat.com/archives/libguestfs/2013-February/msg00102.html

This is cleverer. If it works fine on the various build platforms, we can adopt this.

comment:8 in reply to: ↑ 6 Changed 6 years ago by muks

Replying to marcin:

+m4_define([serial_tests], [
+    m4_esyscmd([automake --version |
+                head -1 |
+                awk '{split ($NF,a,"."); if (a[1] == 1 && a[2] >= 12) { print "serial-tests" }}'
+    ])
+])
+AM_INIT_AUTOMAKE(foreign serial_tests)

It worked fine on my local workstation (Fedora 20). I've made a trac3243_2 branch with this code and submitted it to the builders to test if it works on all builders.

comment:9 Changed 6 years ago by muks

It passed configure on the builders successfully and behaves as expected.

Kean: If you are satisfied with trac3243_2 (described above), please go ahead and merge it to master and close this ticket.

comment:10 Changed 6 years ago by kean

  • Resolution set to fixed
  • Status changed from reviewing to closed

I don't personally like this approach as it gives the user no option of deciding which test harness they want to run (there are good reasons why automake is moving away from the serial test harness). But I don't live with the consequences of this as much as other people who seem to prefer this mechanism, so I am happy to commit it. Merged and closed.

comment:11 Changed 6 years ago by kean

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:12 follow-up: Changed 6 years ago by kean

Re-opening this. After living with this change for just a handful of bugs, the serial harness is just plain getting in the way. I always run "make -j12 check". The serial harness makes this completely untenable. You need to preserve at least SOME mechanism for using the parallel harness. It is my contention that this should be the default, and in the cases where you *need* the serial harness, such as for log scraping on the builders, that the driving scripts somehow enable that. However for developers, I think the parallel harness is more useful. Not only can you run the checks quicker, thus saving time, but if there is a failure and you happened to have just run "make check" without redirecting output, that there are log files present with the results that you can instantly refer to, no need to rerun tests with I/O redirection in place.

I suggest making use of an environment variable in the check, and if set, then enable the serial harness, otherwise use whatever is the default harness assigned by automake. This is less likely to cause any problems with future versions of automake.

comment:13 Changed 6 years ago by kean

  • Owner changed from kean to muks
  • Status changed from reopened to assigned

comment:14 in reply to: ↑ 12 Changed 6 years ago by muks

  • Owner changed from muks to kean
  • Status changed from assigned to reviewing

Hi Kean

  • Fedora 19 introduced automake with the new parallel harness feature about 6 months ago. All these years in BIND 10 development, tests have been run serially on developers' workstations, and are still run serially on builders on various platforms. The parallel harness feature is still not available on many platforms. Serial runs is the status quo.
  • There are very few directories where there are more than 1 test program to be run. So using the parallel harness would not provide any significant benefit at all:

With serial harness and make check:

real  4m13.827s
user  1m4.766s
sys   0m4.632s

With parallel harness and make -j8 check:

real 4m8.498s
user 1m11.212s
sys  0m5.464s

Note that it is still possible to run make -j8 check with the serial
harness. You'd get intermixed output where there are multiple test
programs in the same directory.

  • Our unittests have not been written thinking that they would be run in parallel with other unittests. We share the same resources (such as databases) in many test programs. Concurrency might cause problems.

Taking into account all of the above, I feel what's in master is correct.

comment:15 Changed 6 years ago by marcin

+1 for muks. The DHCP tests were not meant to be run in parallel. Also, I disagree that developers will mostly use parallel test harness. When developing a code, I am constantly running a subset of tests covering the feature I am working on, not the whole test harness. Why would I bother running it in parallel?

Not sure, but I believe that buildbot is not ready to run parallel tests and gather many log files from various directories.

comment:16 Changed 6 years ago by kean

  • Resolution set to fixed
  • Status changed from reviewing to closed

Ok I am convinced!

Note: See TracTickets for help on using tickets.