diff --git a/.travis.yml b/.travis.yml index ab0e9e8c99a..e8949e621ce 100644 --- a/.travis.yml +++ b/.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 diff --git a/CMakeLists.txt b/CMakeLists.txt index 235a620616e..d142d0b698b 100644 --- a/CMakeLists.txt +++ b/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}" $<$:MIXXX_BUILD_DEBUG> + $<$:MIXXX_DEBUG_ASSERTIONS_ENABLED> $<$>: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 ) diff --git a/build/depends.py b/build/depends.py index 4ab669f9fdd..32f5a38c059 100644 --- a/build/depends.py +++ b/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 diff --git a/src/library/dao/autodjcratesdao.cpp b/src/library/dao/autodjcratesdao.cpp index ec6655a6a89..58809c2baa4 100644 --- a/src/library/dao/autodjcratesdao.cpp +++ b/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 diff --git a/src/test/mixxxtest.cpp b/src/test/mixxxtest.cpp index a57debe7c78..07dbf28da6f 100644 --- a/src/test/mixxxtest.cpp +++ b/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 MixxxTest::s_pApplication; @@ -24,6 +26,19 @@ 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. @@ -31,6 +46,7 @@ MixxxTest::ApplicationScope::ApplicationScope(int& argc, char** argv) { } MixxxTest::ApplicationScope::~ApplicationScope() { + mixxx::Logging::shutdown(); DEBUG_ASSERT(s_pApplication); s_pApplication.reset(); } diff --git a/src/util/assert.h b/src/util/assert.h index 51ee2b7f7ef..0983e03e9d0 100644 --- a/src/util/assert.h +++ b/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) diff --git a/src/util/cmdlineargs.h b/src/util/cmdlineargs.h index 2df0a020121..22ce3099dbd 100644 --- a/src/util/cmdlineargs.h +++ b/src/util/cmdlineargs.h @@ -1,16 +1,20 @@ -#ifndef CMDLINEARGS_H -#define CMDLINEARGS_H +#pragma once +#include +#include #include #include -#include -#include #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 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 */ diff --git a/src/util/logging.cpp b/src/util/logging.cpp index 9e29712cf7a..73d4961fab3 100644 --- a/src/util/logging.cpp +++ b/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. diff --git a/src/util/logging.h b/src/util/logging.h index 79f71eb9279..98af667665f 100644 --- a/src/util/logging.h +++ b/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);