After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 679826 - Investigate using parallel-tests to speed up testing
Investigate using parallel-tests to speed up testing
Status: RESOLVED FIXED
Product: folks
Classification: Platform
Component: general
git master
Other All
: Normal normal
: Unset
Assigned To: folks-maint
folks-maint
Depends on:
Blocks:
 
 
Reported: 2012-07-12 21:32 UTC by Philip Withnall
Modified: 2013-10-09 22:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Manually serialize eds and tracker tests and use the parallel harness (20.62 KB, patch)
2013-09-25 00:35 UTC, Travis Reitter
none Details | Review
Use autotools' parallel test harness (try 2) (3.60 KB, patch)
2013-09-25 17:29 UTC, Travis Reitter
needs-work Details | Review
Use autotools' parallel test harness (try 3) (7.12 KB, patch)
2013-09-30 17:50 UTC, Travis Reitter
accepted-commit_now Details | Review

Comment 1 Philip Withnall 2012-07-12 21:32:29 UTC
See also: bug #657456.
Comment 2 Jeremy Whiting 2012-07-20 21:42:15 UTC
I spent a few minutes trying this today and founnd the following:

The tracker unit tests aren't paralellizable, if I run them with make -j4 check half of them fail, if I run them with make check they all pass.

The telepathy unit tests set TESTS to $(filter-out blah blah, $(noinst-programs)) so expanding it in place is needed.

Other than the tracker unit tests this should work fine, maybe there's a way to mark the tracker tests as not to run in parallel, I'll check.
Comment 3 Philip Withnall 2012-07-30 09:50:12 UTC
(In reply to comment #2)
> I spent a few minutes trying this today and founnd the following:
> 
> The tracker unit tests aren't paralellizable, if I run them with make -j4 check
> half of them fail, if I run them with make check they all pass.

Any idea why they’re not parallelisable? My guess would be some inherent behaviour in Tracker which we’ll have to file a bug about and work around. Is it possible to explicitly set N=1 for -jN? If not, we’ll have to manually specify that all the Tracker tests are in a linear dependency chain by specifying dependencies on their log files.

> The telepathy unit tests set TESTS to $(filter-out blah blah,
> $(noinst-programs)) so expanding it in place is needed.

Not a problem.
Comment 4 Travis Reitter 2013-09-25 00:35:41 UTC
Created attachment 255653 [details] [review]
Manually serialize eds and tracker tests and use the parallel harness

This enables the parallel harness while forcing the eds and tracker tests to be serial. See the patch for more details.

The aliasing of the test names in the manual serialization is annoying; if anyone has suggestions for a magic rule to cover them all (based on the content of TESTS), I'd greatly appreciate it. I tried using the $(words) function with no luck (I guess it can't be used in rule targets or results).
Comment 5 Travis Reitter 2013-09-25 17:29:32 UTC
Created attachment 255709 [details] [review]
Use autotools' parallel test harness (try 2)

Patch from branch:

http://cgit.collabora.com/git/user/treitter/folks.git/log/?h=bgo679826-parallel-tests-try2

This one is much cleaner and simpler than the last since it uses a single declaration to be non-parallel instead of adding a long, manually-maintained dependency chain.
Comment 6 Philip Withnall 2013-09-28 13:38:07 UTC
Review of attachment 255709 [details] [review]:

Looks good. I think it would be useful to add the *.log files to GITIGNOREFILES (or, better yet, fix upstream git.mk to automatically ignore the log files from parallel-tests).

::: tests/eds/Makefile.am
@@ +1,3 @@
+# This forces serialization of the tests because they run into terrible race
+# conditions if run in parallel (ie, make -jN, n > 1)
+.NOTPARALLEL:

Might be an idea to add a FIXME here pointing to a bug about making the EDS tests parallelisable.

::: tests/tracker/Makefile.am
@@ +1,3 @@
+# This forces serialization of the tests because they run into terrible race
+# conditions if run in parallel (ie, make -jN, n > 1)
+.NOTPARALLEL:

And here.

@@ +108,3 @@
+	match-phone-number \
+	match-name \
+	match-all \

This should probably be in a separate whitespace cleanup commit (which could be pushed immediately). Otherwise it could be confusing.
Comment 7 Travis Reitter 2013-09-30 17:50:42 UTC
Created attachment 256124 [details] [review]
Use autotools' parallel test harness (try 3)

This should address all the review comments. Most of the git.mk changes had already been done upstream and Behdad did the remaining pieces just after I filed an issue, so it's all upstreamed and downstreamed. No need to clutter our Makefiles.

In fact, he pointed out that we could simplify our configure.ac even further (which I have done).

I also filed and cited the recommended bugs.
Comment 8 Philip Withnall 2013-10-03 14:52:36 UTC
Review of attachment 256124 [details] [review]:

Looks good to me. Don’t forget NEWS.
Comment 9 Travis Reitter 2013-10-09 22:22:42 UTC
After some kerfuffle with the state of my build prefix, this branch once again passes tests consistently for me, so I added a NEWS entry and merged it:

commit fd6f44d35663405fc93891a39a071fd6bedeecce
Author: Travis Reitter <travis.reitter@collabora.co.uk>
Date:   Tue Sep 24 16:23:46 2013 -0700

    Use autotools's parallel test harness
    
    We were already unintentionally using it in some cases (depending upon the
    developer's system) without the proper safeguards in place. Without them,
    the EDS and Tracker tests will mysteriously result in a ton of segfaulting
    background processes because they can't be safely run simultaneously.
    
    The upside of using the parallel harness is that it's still safe to use for
    the simpler test suites (eg, key-file, folks, telepathy) and should speed up
    their runtimes quite a bit. It also adds cleaner logging facilities and
    makes the default output a little cleaner.
    
    Bug: https://bugzilla.gnome.org/show_bug.cgi?id=679826

 NEWS                      | 1 +
 configure.ac              | 3 ++-
 tests/eds/Makefile.am     | 6 ++++++
 tests/tracker/Makefile.am | 6 ++++++
 4 files changed, 15 insertions(+), 1 deletion(-)