[Libreoffice-commits] core.git: external/cppunit

[Stephan Bergmann (via logerrit)]

 external/cppunit/UnpackedTarball_cppunit.mk |    1 +
 external/cppunit/order.patch.0              |   25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+)

New commits:
commit 2f2246d22e2a8ccbc1dc3e6f5243734a61edf270
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Sat Aug 8 12:00:23 2020 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Sat Aug 8 14:18:54 2020 +0200

    external/cppunit: Run tests in deterministic order
    
    What prompted me to make this change is that when I tried an --enable-lto build,
    CppunitTest_xmlsecurity_signing failed in a non-obvious way, and I noticed that
    it ran the individual tests in a different order than a --disable-lto build.
    
    With this change, CppunitTest_xmlsecurity_signing also started to fail in the
    --disable-lto build, which has meanwhile been fixed with
    800eebfa82106c509310ed43bef38a7a4ad4451f "Database document apparently needs to
    be closed before it is disposed".  No other tests started to fail in my
    Linux --disable-lto-build with this change, and my Linux --enable-lto build
    (using recent Fedora 32 GCC 10.2 and the default linker) also succeeds `make
    check` after 800eebfa82106c509310ed43bef38a7a4ad4451f, both with and without
    this cppunit change.
    (<https://bugs.documentfoundation.org/show_bug.cgi?id=126442> "LTO build
    segfaults in sw_apitests" and <https://src.fedoraproject.org/rpms/libreoffice/c/
    5d644f1606b76ffa4a102433849a824d7293a404> "%check fails with lto enabled"
    indicate that older branches also fail CppunitTest_sw_apitests with
    --enable-lto, but I could not reproduce that on current master.)
    
    What happens in cppunit is that every CPPUNIT_TEST_SUITE_REGISTRATION (or other
    macro like CPPUNIT_TEST_FIXTURE internally calling
    CPPUNIT_TEST_SUITE_REGISTRATION) creates a global static variable whose ctor
    inserts the address of a sub-object of that global static variable into the
    TestFactoryRegistry::m_factories set.  Even if the order of invocation of those
    ctors from one .cxx is deterministic, the relative order or the addresses of
    those sub-objects inserted into the TestFactoryRegistry::m_factories set need
    not be (though they probably typically are).  Another source of nondeterminism
    is that the order of ctors from different .cxx is not specified (which might
    have caused the CppunitTest_xmlsecurity_signing failures, given that test
    includes suites from two different .cxx).
    
    So to make test execution more reproducible, make the order in which the tests
    are run deterministic, sorting them by name.  (When
    TestFactoryRegistry::addTestToSuite the adds the sorted tests to
    TestSuite::addTest, they are inserted into a TestSuite::m_tests vector, from
    which point on things appear to already happen in a deterministic order.)
    
    Change-Id: I40741f397a96772974fd41bacdb3dd763c885417
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/100384
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/external/cppunit/UnpackedTarball_cppunit.mk b/external/cppunit/UnpackedTarball_cppunit.mk
index 24f75b43415f..c645c2bf4db3 100644
--- a/external/cppunit/UnpackedTarball_cppunit.mk
+++ b/external/cppunit/UnpackedTarball_cppunit.mk
@@ -19,6 +19,7 @@ $(eval $(call gb_UnpackedTarball_add_patches,cppunit,\
 	external/cppunit/CPPUNIT_PLUGIN_EXPORT.patch.0 \
 	external/cppunit/enable-win32-debug.patch \
 	external/cppunit/rtti.patch.0 \
+	external/cppunit/order.patch.0 \
 ))
 ifeq ($(DISABLE_DYNLOADING),TRUE)
 $(eval $(call gb_UnpackedTarball_add_patches,cppunit,\
diff --git a/external/cppunit/order.patch.0 b/external/cppunit/order.patch.0
new file mode 100644
index 000000000000..523b3cd704e1
--- /dev/null
+++ b/external/cppunit/order.patch.0
@@ -0,0 +1,25 @@
+--- src/cppunit/TestFactoryRegistry.cpp
++++ src/cppunit/TestFactoryRegistry.cpp
+@@ -143,13 +143,21 @@
+ void 
+ TestFactoryRegistry::addTestToSuite( TestSuite *suite )
+ {
++  std::multimap<std::string, Test *> sorted;
+   for ( Factories::iterator it = m_factories.begin(); 
+         it != m_factories.end(); 
+         ++it )
+   {
+     TestFactory *factory = *it;
+-    suite->addTest( factory->makeTest() );
++    Test *test = factory->makeTest();
++    sorted.insert({test->getName(), test});
+   }
++  // In the unlikely case of multiple Tests with identical names, those will
++  // still be added in random order:
++  for (auto const &i: sorted)
++  {
++    suite->addTest( i.second );
++  }
+ }
+ 
+