test: Break if DEBUG_ASSERT is encountered

.travis.yml

@@ -69,7 +69,7 @@ jobs:
         # NOTE(sblaisot): 2018-01-02 removing gdb wrapper on linux due to a bug in
         # return code in order to avoid having a successful build when a test fail.
         # https://bugs.launchpad.net/mixxx/+bug/1699689
-        - ./mixxx-test
+        - ./mixxx-test --logLevel info
 
     - name: Ubuntu/gcc/CMake build
       compiler: gcc
@@ -144,7 +144,7 @@ jobs:
       script:
         # lldb doesn't provide an easy way to exit 1 on error:
         # https://bugs.llvm.org/show_bug.cgi?id=27326
-        - lldb ./mixxx-test --batch -o run -o quit -k 'thread backtrace all' -k "script import os; os._exit(1)"
+        - lldb ./mixxx-test --batch -o run -o quit -k 'thread backtrace all' -k "script import os; os._exit(1)" -- --logLevel info
       before_cache:
         # Avoid indefinite cache growth
         - brew cleanup

CMakeLists.txt

@@ -954,12 +954,16 @@ endif()
 
 option(DEBUG_ASSERTIONS_FATAL "Fail if debug become true assertions" OFF)
 if(DEBUG_ASSERTIONS_FATAL)
-  target_compile_definitions(mixxx-lib PUBLIC MIXXX_DEBUG_ASSERTIONS_FATAL)
+  target_compile_definitions(mixxx-lib PUBLIC MIXXX_DEBUG_ASSERTIONS_FATAL MIXXX_DEBUG_ASSERTIONS_ENABLED)
+  if (NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
+    message(STATUS "DEBUG_ASSERT statements have been enabled because DEBUG_ASSERTIONS_FATAL is ON.")
+  endif()
 endif()
 
 target_compile_definitions(mixxx-lib PUBLIC
   "${CMAKE_SYSTEM_PROCESSOR}"
   $<$<CONFIG:Debug>:MIXXX_BUILD_DEBUG>
+  $<$<CONFIG:Debug>:MIXXX_DEBUG_ASSERTIONS_ENABLED>
   $<$<NOT:$<CONFIG:Debug>>:MIXXX_BUILD_RELEASE>
   # Disable assert.h assertions in release mode. Some libraries use
   # this as a signal for when to enable code that should be disabled
@@ -1328,6 +1332,7 @@ include(GoogleTest)
 enable_testing()
 gtest_add_tests(
   TARGET mixxx-test
+  EXTRA_ARGS --logLevel info
   WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
   TEST_LIST testsuite
 )

build/depends.py

@@ -1427,6 +1427,7 @@ def configure(self, build, conf):
 
         if build.build_is_debug:
             build.env.Append(CPPDEFINES='MIXXX_BUILD_DEBUG')
+            build.env.Append(CPPDEFINES='MIXXX_DEBUG_ASSERTIONS_ENABLED')
         elif build.build_is_release:
             build.env.Append(CPPDEFINES='MIXXX_BUILD_RELEASE')
             # Disable assert.h assertions in release mode. Some libraries use
@@ -1450,6 +1451,7 @@ def configure(self, build, conf):
 
         if int(SCons.ARGUMENTS.get('debug_assertions_fatal', 0)):
             build.env.Append(CPPDEFINES='MIXXX_DEBUG_ASSERTIONS_FATAL')
+            build.env.Append(CPPDEFINES='MIXXX_DEBUG_ASSERTIONS_ENABLED')
 
         if build.toolchain_is_gnu:
             # Default GNU Options

src/library/dao/autodjcratesdao.cpp

@@ -39,7 +39,7 @@ namespace {
 const int kLeastPreferredPercent = 15;
 
 // These consts are only used for DEBUG_ASSERTs
-#ifdef MIXXX_BUILD_DEBUG
+#ifdef MIXXX_DEBUG_ASSERTIONS_ENABLED
 const int kLeastPreferredPercentMin = 0;
 const int kLeastPreferredPercentMax = 50;
 #endif

src/test/mixxxtest.cpp

@@ -2,6 +2,8 @@
 
 #include "library/coverartutils.h"
 #include "sources/soundsourceproxy.h"
+#include "util/cmdlineargs.h"
+#include "util/logging.h"
 
 namespace {
 
@@ -12,7 +14,7 @@ QString makeTestConfigFile(const QString& path) {
     return path;
 }
 
-}  // namespace
+} // namespace
 
 // Static initialization
 QScopedPointer<MixxxApplication> MixxxTest::s_pApplication;
@@ -24,13 +26,27 @@ MixxxTest::ApplicationScope::ApplicationScope(int& argc, char** argv) {
 
     SoundSourceProxy::registerSoundSourceProviders();
 
+    // Construct a list of strings based on the command line arguments
+    CmdlineArgs args;
+    DEBUG_ASSERT(args.Parse(argc, argv));
+    mixxx::LogLevel logLevel = args.getLogLevel();
+
+    // Log level Debug would produce too many log messages that
+    // might abort and fail the CI builds.
+    mixxx::Logging::initialize(
+            QDir(), // No log file should be written during tests, only output to stderr
+            logLevel,
+            logLevel,
+            true);
+
     // All guessing of cover art should be done synchronously
     // in the same thread during tests to prevent test failures
     // due to timing issues.
     disableConcurrentGuessingOfTrackCoverInfoDuringTests();
 }
 
 MixxxTest::ApplicationScope::~ApplicationScope() {
+    mixxx::Logging::shutdown();
     DEBUG_ASSERT(s_pApplication);
     s_pApplication.reset();
 }

src/util/assert.h

@@ -12,7 +12,7 @@ inline void mixxx_debug_assert(const char* assertion, const char* file, int line
 }
 
 inline bool mixxx_maybe_debug_assert_return_true(const char* assertion, const char* file, int line, const char* function) {
-#ifdef MIXXX_BUILD_DEBUG
+#ifdef MIXXX_DEBUG_ASSERTIONS_ENABLED
     mixxx_debug_assert(assertion, file, line, function);
 #else
     Q_UNUSED(assertion);
@@ -51,7 +51,7 @@ inline void mixxx_release_assert(const char* assertion, const char* file, int li
 //   DEBUG_ASSERT(doSomething());
 //
 // In release builds, doSomething() is never called!
-#ifdef MIXXX_BUILD_DEBUG
+#ifdef MIXXX_DEBUG_ASSERTIONS_ENABLED
 #define DEBUG_ASSERT(cond) ((!(cond)) ? mixxx_debug_assert(#cond, __FILE__, __LINE__, ASSERT_FUNCTION) : mixxx_noop())
 #else
 #define DEBUG_ASSERT(cond)

src/util/cmdlineargs.h

@@ -1,16 +1,20 @@
-#ifndef CMDLINEARGS_H
-#define CMDLINEARGS_H
+#pragma once
 
+#include <QDesktopServices>
+#include <QDir>
 #include <QList>
 #include <QString>
-#include <QDir>
-#include <QDesktopServices>
 
 #include "util/logging.h"
 
-// A structure to store the parsed command-line arguments
+/// A structure to store the parsed command-line arguments
 class CmdlineArgs final {
   public:
+    /// The constructor is only public to make this class reusable in tests.
+    /// All operational code in Mixxx itself must access the global singleton
+    /// via `CmdlineArgs::instance()`.
+    CmdlineArgs();
+
     static inline CmdlineArgs& Instance() {
         static CmdlineArgs cla;
         return cla;
@@ -39,8 +43,6 @@ class CmdlineArgs final {
     const QString& getTimelinePath() const { return m_timelinePath; }
 
   private:
-    CmdlineArgs();
-
     QList<QString> m_musicFiles;    // List of files to load into players at startup
     bool m_startInFullscreen;       // Start in fullscreen mode
     bool m_midiDebug;
@@ -56,5 +58,3 @@ class CmdlineArgs final {
     QString m_pluginPath;
     QString m_timelinePath;
 };
-
-#endif /* CMDLINEARGS_H */

src/util/logging.cpp

@@ -148,46 +148,54 @@ void MessageHandler(QtMsgType type,
 }  // namespace
 
 // static
-void Logging::initialize(const QDir& settingsDir,
-                         LogLevel logLevel,
-                         LogLevel logFlushLevel,
-                         bool debugAssertBreak) {
+void Logging::initialize(
+        const QDir& logDir,
+        LogLevel logLevel,
+        LogLevel logFlushLevel,
+        bool debugAssertBreak) {
     VERIFY_OR_DEBUG_ASSERT(!g_logfile.isOpen()) {
         // Somebody already called Logging::initialize.
         return;
     }
 
     setLogLevel(logLevel);
-    g_logFlushLevel = logFlushLevel;
 
-    QString logFileName;
+    if (logDir.exists()) {
+        QString logFileName;
 
-    // Rotate old logfiles.
-    for (int i = 9; i >= 0; --i) {
-        if (i == 0) {
-            logFileName = settingsDir.filePath("mixxx.log");
-        } else {
-            logFileName = settingsDir.filePath(QString("mixxx.log.%1").arg(i));
-        }
-        QFileInfo logbackup(logFileName);
-        if (logbackup.exists()) {
-            QString olderlogname =
-                    settingsDir.filePath(QString("mixxx.log.%1").arg(i + 1));
-            // This should only happen with number 10
-            if (QFileInfo::exists(olderlogname)) {
-                QFile::remove(olderlogname);
+        // Rotate old logfiles.
+        for (int i = 9; i >= 0; --i) {
+            if (i == 0) {
+                logFileName = logDir.filePath("mixxx.log");
+            } else {
+                logFileName = logDir.filePath(QString("mixxx.log.%1").arg(i));
             }
-            if (!QFile::rename(logFileName, olderlogname)) {
-                fprintf(stderr, "Error rolling over logfile %s",
-                        logFileName.toLocal8Bit().constData());
+            QFileInfo logbackup(logFileName);
+            if (logbackup.exists()) {
+                QString olderlogname =
+                        logDir.filePath(QString("mixxx.log.%1").arg(i + 1));
+                // This should only happen with number 10
+                if (QFileInfo::exists(olderlogname)) {
+                    QFile::remove(olderlogname);
+                }
+                if (!QFile::rename(logFileName, olderlogname)) {
+                    fprintf(stderr,
+                            "Error rolling over logfile %s",
+                            logFileName.toLocal8Bit().constData());
+                }
             }
         }
+        // Since the message handler is not installed yet, we can touch s_logfile
+        // without the lock.
+        g_logfile.setFileName(logFileName);
+        g_logfile.open(QIODevice::WriteOnly | QIODevice::Text);
+        g_logFlushLevel = logFlushLevel;
+    } else {
+        qInfo() << "No directory for writing a log file";
+        // No need to flush anything
+        g_logFlushLevel = LogLevel::Critical;
     }
 
-    // Since the message handler is not installed yet, we can touch g_logfile
-    // without the lock.
-    g_logfile.setFileName(logFileName);
-    g_logfile.open(QIODevice::WriteOnly | QIODevice::Text);
     g_debugAssertBreak = debugAssertBreak;
 
     // Install the Qt message handler.

src/util/logging.h

@@ -27,10 +27,11 @@ extern LogLevel g_logFlushLevel;
 class Logging {
   public:
     // These are not thread safe. Only call them on Mixxx startup and shutdown.
-    static void initialize(const QDir& settingsDir,
-                           LogLevel logLevel,
-                           LogLevel logFlushLevel,
-                           bool debugAssertBreak);
+    static void initialize(
+            const QDir& logDir,
+            LogLevel logLevel,
+            LogLevel logFlushLevel,
+            bool debugAssertBreak);
 
     // Sets only the loglevel without the on-disk settings.  Used by mixxx-test.
     static void setLogLevel(LogLevel logLevel);