From 118c3e4aca3d7f71e9313b83762fe3b0cf48d28e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 7 Oct 2019 19:28:41 +0200 Subject: [PATCH] Audio: 100% test coverage for the base Importer class. One bug found. --- src/Magnum/Audio/AbstractImporter.cpp | 20 +- src/Magnum/Audio/AbstractImporter.h | 8 + src/Magnum/Audio/CMakeLists.txt | 54 ++- .../Audio/Test/AbstractImporterTest.cpp | 320 ++++++++++++++++-- src/Magnum/Audio/Test/CMakeLists.txt | 2 +- src/Magnum/Audio/visibility.h | 2 +- 6 files changed, 381 insertions(+), 25 deletions(-) diff --git a/src/Magnum/Audio/AbstractImporter.cpp b/src/Magnum/Audio/AbstractImporter.cpp index c97dc26ca..ec92e68c3 100644 --- a/src/Magnum/Audio/AbstractImporter.cpp +++ b/src/Magnum/Audio/AbstractImporter.cpp @@ -26,6 +26,7 @@ #include "AbstractImporter.h" #include +#include #include #include #include @@ -100,7 +101,7 @@ void AbstractImporter::doOpenFile(const std::string& filename) { /* Open file */ if(!Utility::Directory::exists(filename)) { - Error() << "Trade::AbstractImporter::openFile(): cannot open file" << filename; + Error() << "Audio::AbstractImporter::openFile(): cannot open file" << filename; return; } @@ -129,4 +130,21 @@ Containers::Array AbstractImporter::data() { return doData(); } +Debug& operator<<(Debug& debug, const AbstractImporter::Feature value) { + switch(value) { + /* LCOV_EXCL_START */ + #define _c(v) case AbstractImporter::Feature::v: return debug << "Audio::AbstractImporter::Feature::" #v; + _c(OpenData) + #undef _c + /* LCOV_EXCL_STOP */ + } + + return debug << "Audio::AbstractImporter::Feature(" << Debug::nospace << reinterpret_cast(UnsignedByte(value)) << Debug::nospace << ")"; +} + +Debug& operator<<(Debug& debug, const AbstractImporter::Features value) { + return Containers::enumSetDebugOutput(debug, value, "Audio::AbstractImporter::Features{}", { + AbstractImporter::Feature::OpenData}); +} + }} diff --git a/src/Magnum/Audio/AbstractImporter.h b/src/Magnum/Audio/AbstractImporter.h index 95010fa70..94d39c13b 100644 --- a/src/Magnum/Audio/AbstractImporter.h +++ b/src/Magnum/Audio/AbstractImporter.h @@ -193,6 +193,14 @@ class MAGNUM_AUDIO_EXPORT AbstractImporter: public PluginManager::AbstractManagi virtual Containers::Array doData() = 0; }; +CORRADE_ENUMSET_OPERATORS(AbstractImporter::Features) + +/** @debugoperatorclassenum{AbstractImporter,AbstractImporter::Feature} */ +MAGNUM_AUDIO_EXPORT Debug& operator<<(Debug& debug, AbstractImporter::Feature value); + +/** @debugoperatorclassenum{AbstractImporter,AbstractImporter::Features} */ +MAGNUM_AUDIO_EXPORT Debug& operator<<(Debug& debug, AbstractImporter::Features value); + }} #endif diff --git a/src/Magnum/Audio/CMakeLists.txt b/src/Magnum/Audio/CMakeLists.txt index f269f0c3b..c6665d232 100644 --- a/src/Magnum/Audio/CMakeLists.txt +++ b/src/Magnum/Audio/CMakeLists.txt @@ -27,7 +27,6 @@ find_package(Corrade REQUIRED PluginManager) find_package(OpenAL REQUIRED) set(MagnumAudio_SRCS - AbstractImporter.cpp Audio.cpp Buffer.cpp BufferFormat.cpp @@ -35,6 +34,9 @@ set(MagnumAudio_SRCS Renderer.cpp Source.cpp) +set(MagnumAudio_GracefulAssert_SRCS + AbstractImporter.cpp) + set(MagnumAudio_HEADERS AbstractImporter.h Audio.h @@ -64,10 +66,26 @@ if(WITH_SCENEGRAPH) PlayableGroup.cpp) endif() -# Audio library -add_library(MagnumAudio ${SHARED_OR_STATIC} +# Objects shared between main and test library +add_library(MagnumAudioObjects OBJECT ${MagnumAudio_SRCS} ${MagnumAudio_HEADERS}) +target_include_directories(MagnumAudioObjects PUBLIC + ${OPENAL_INCLUDE_DIR} + $ + $) +if(NOT BUILD_STATIC) + target_compile_definitions(MagnumAudioObjects PRIVATE "MagnumAudioObjects_EXPORTS") +endif() +if(NOT BUILD_STATIC OR BUILD_STATIC_PIC) + set_target_properties(MagnumAudioObjects PROPERTIES POSITION_INDEPENDENT_CODE ON) +endif() +set_target_properties(MagnumAudioObjects PROPERTIES FOLDER "Magnum/Audio") + +# Audio library +add_library(MagnumAudio ${SHARED_OR_STATIC} + $ + ${MagnumAudio_GracefulAssert_SRCS}) target_include_directories(MagnumAudio PUBLIC ${OPENAL_INCLUDE_DIR}) set_target_properties(MagnumAudio PROPERTIES DEBUG_POSTFIX "-d" @@ -123,5 +141,35 @@ if(WITH_AL_INFO) endif() if(BUILD_TESTS) + # Library with graceful assert for testing + add_library(MagnumAudioTestLib ${SHARED_OR_STATIC} + $ + ${MagnumAudio_GracefulAssert_SRCS}) + target_compile_definitions(MagnumAudioTestLib PRIVATE + "CORRADE_GRACEFUL_ASSERT" "MagnumAudio_EXPORTS") + target_include_directories(MagnumAudioTestLib PUBLIC ${OPENAL_INCLUDE_DIR}) + set_target_properties(MagnumAudioTestLib PROPERTIES + DEBUG_POSTFIX "-d" + FOLDER "Magnum") + if(BUILD_STATIC_PIC) + set_target_properties(MagnumAudioTestLib PROPERTIES POSITION_INDEPENDENT_CODE ON) + endif() + target_link_libraries(MagnumAudioTestLib + Magnum + Corrade::PluginManager + ${OPENAL_LIBRARY}) + if(WITH_SCENEGRAPH) + target_link_libraries(MagnumAudioTestLib MagnumSceneGraph) + endif() + + # On Windows we need to install first and then run the tests to avoid "DLL + # not found" hell, thus we need to install this too + if(CORRADE_TARGET_WINDOWS AND NOT CMAKE_CROSSCOMPILING AND NOT BUILD_STATIC) + install(TARGETS MagnumAudioTestLib + RUNTIME DESTINATION ${MAGNUM_BINARY_INSTALL_DIR} + LIBRARY DESTINATION ${MAGNUM_LIBRARY_INSTALL_DIR} + ARCHIVE DESTINATION ${MAGNUM_LIBRARY_INSTALL_DIR}) + endif() + add_subdirectory(Test) endif() diff --git a/src/Magnum/Audio/Test/AbstractImporterTest.cpp b/src/Magnum/Audio/Test/AbstractImporterTest.cpp index 552f35430..8c3736d67 100644 --- a/src/Magnum/Audio/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Audio/Test/AbstractImporterTest.cpp @@ -23,11 +23,15 @@ DEALINGS IN THE SOFTWARE. */ +#include #include #include +#include +#include #include #include "Magnum/Audio/AbstractImporter.h" +#include "Magnum/Audio/BufferFormat.h" #include "configure.h" @@ -36,39 +40,317 @@ namespace Magnum { namespace Audio { namespace Test { namespace { struct AbstractImporterTest: TestSuite::Tester { explicit AbstractImporterTest(); - void openFile(); + void construct(); + + void openData(); + void openFileAsData(); + void openFileAsDataNotFound(); + + void openFileNotImplemented(); + void openDataNotSupported(); + void openDataNotImplemented(); + + /* file callbacks not supported -- those will be once this gets merged with + Trade::AbstractImporter */ + + void format(); + void formatNoFile(); + + void frequency(); + void frequencyNoFile(); + + void data(); + void dataNoFile(); + + void debugFeature(); + void debugFeatures(); }; AbstractImporterTest::AbstractImporterTest() { - addTests({&AbstractImporterTest::openFile}); + addTests({&AbstractImporterTest::construct, + + &AbstractImporterTest::openData, + &AbstractImporterTest::openFileAsData, + &AbstractImporterTest::openFileAsDataNotFound, + + &AbstractImporterTest::openFileNotImplemented, + &AbstractImporterTest::openDataNotSupported, + &AbstractImporterTest::openDataNotImplemented, + + &AbstractImporterTest::format, + &AbstractImporterTest::formatNoFile, + + &AbstractImporterTest::frequency, + &AbstractImporterTest::frequencyNoFile, + + &AbstractImporterTest::data, + &AbstractImporterTest::dataNoFile, + + &AbstractImporterTest::debugFeature, + &AbstractImporterTest::debugFeatures}); } -void AbstractImporterTest::openFile() { - class DataImporter: public Audio::AbstractImporter { - public: - explicit DataImporter(): opened(false) {} +void AbstractImporterTest::construct() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return false; } + void doClose() override {} + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { return nullptr; } + } importer; + + CORRADE_COMPARE(importer.features(), AbstractImporter::Features{}); + CORRADE_VERIFY(!importer.isOpened()); + + importer.close(); + CORRADE_VERIFY(!importer.isOpened()); +} + +void AbstractImporterTest::openData() { + struct: AbstractImporter { + Features doFeatures() const override { return Feature::OpenData; } + bool doIsOpened() const override { return _opened; } + void doClose() override { _opened = false; } + + void doOpenData(Containers::ArrayView data) override { + _opened = (data.size() == 1 && data[0] == '\xa5'); + } + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { return nullptr; } - private: - Features doFeatures() const override { return Feature::OpenData; } - bool doIsOpened() const override { return opened; } - void doClose() override {} + bool _opened = false; + } importer; - void doOpenData(Containers::ArrayView data) override { - opened = (data.size() == 1 && data[0] == '\xa5'); - } + CORRADE_VERIFY(!importer.isOpened()); + const char a5 = '\xa5'; + CORRADE_VERIFY(importer.openData({&a5, 1})); + CORRADE_VERIFY(importer.isOpened()); + + importer.close(); + CORRADE_VERIFY(!importer.isOpened()); +} + +void AbstractImporterTest::openFileAsData() { + struct: AbstractImporter { + Features doFeatures() const override { return Feature::OpenData; } + bool doIsOpened() const override { return _opened; } + void doClose() override { _opened = false; } - BufferFormat doFormat() const override { return {}; } - UnsignedInt doFrequency() const override { return {}; } - Corrade::Containers::Array doData() override { return nullptr; } + void doOpenData(Containers::ArrayView data) override { + _opened = (data.size() == 1 && data[0] == '\xa5'); + } - bool opened; - }; + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { return nullptr; } + + bool _opened = false; + } importer; /* doOpenFile() should call doOpenData() */ - DataImporter importer; CORRADE_VERIFY(!importer.isOpened()); importer.openFile(Utility::Directory::join(AUDIO_TEST_DIR, "file.bin")); CORRADE_VERIFY(importer.isOpened()); + + importer.close(); + CORRADE_VERIFY(!importer.isOpened()); +} + +void AbstractImporterTest::openFileAsDataNotFound() { + struct Importer: AbstractImporter { + Features doFeatures() const override { return Feature::OpenData; } + bool doIsOpened() const override { return _opened; } + void doClose() override { _opened = false; } + + void doOpenData(Containers::ArrayView) override { + _opened = true; + } + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { return nullptr; } + + bool _opened = false; + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + CORRADE_VERIFY(!importer.openFile("nonexistent.bin")); + CORRADE_VERIFY(!importer.isOpened()); + CORRADE_COMPARE(out.str(), "Audio::AbstractImporter::openFile(): cannot open file nonexistent.bin\n"); +} + +void AbstractImporterTest::openFileNotImplemented() { + struct Importer: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return false; } + void doClose() override {} + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { return nullptr; } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + CORRADE_VERIFY(!importer.openFile("file.dat")); + CORRADE_COMPARE(out.str(), "Audio::AbstractImporter::openFile(): not implemented\n"); +} + +void AbstractImporterTest::openDataNotSupported() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return false; } + void doClose() override {} + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { return nullptr; } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + CORRADE_VERIFY(!importer.openData(nullptr)); + CORRADE_COMPARE(out.str(), "Audio::AbstractImporter::openData(): feature not supported\n"); +} + +void AbstractImporterTest::openDataNotImplemented() { + struct: AbstractImporter { + Features doFeatures() const override { return Feature::OpenData; } + bool doIsOpened() const override { return false; } + void doClose() override {} + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { return nullptr; } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + CORRADE_VERIFY(!importer.openData(nullptr)); + CORRADE_COMPARE(out.str(), "Audio::AbstractImporter::openData(): feature advertised but not implemented\n"); +} + +void AbstractImporterTest::format() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + BufferFormat doFormat() const override { return BufferFormat::Mono8; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { return nullptr; } + } importer; + + CORRADE_COMPARE(importer.format(), BufferFormat::Mono8); +} + +void AbstractImporterTest::formatNoFile() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return false; } + void doClose() override {} + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { return nullptr; } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + importer.format(); + CORRADE_COMPARE(out.str(), "Audio::AbstractImporter::format(): no file opened\n"); +} + +void AbstractImporterTest::frequency() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return 44000; } + Containers::Array doData() override { return nullptr; } + } importer; + + CORRADE_COMPARE(importer.frequency(), 44000); +} + +void AbstractImporterTest::frequencyNoFile() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return false; } + void doClose() override {} + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { return nullptr; } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + importer.frequency(); + CORRADE_COMPARE(out.str(), "Audio::AbstractImporter::frequency(): no file opened\n"); +} + +void AbstractImporterTest::data() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { + Containers::Array out{1}; + out[0] = 'H'; + return out; + } + } importer; + + CORRADE_COMPARE_AS(importer.data(), (Containers::Array{Containers::InPlaceInit, {'H'}}), TestSuite::Compare::Container); +} + +void AbstractImporterTest::dataNoFile() { + struct: AbstractImporter { + Features doFeatures() const override { return {}; } + bool doIsOpened() const override { return false; } + void doClose() override {} + + BufferFormat doFormat() const override { return {}; } + UnsignedInt doFrequency() const override { return {}; } + Containers::Array doData() override { return nullptr; } + } importer; + + std::ostringstream out; + Error redirectError{&out}; + + importer.data(); + CORRADE_COMPARE(out.str(), "Audio::AbstractImporter::data(): no file opened\n"); +} + +void AbstractImporterTest::debugFeature() { + std::ostringstream out; + + Debug{&out} << AbstractImporter::Feature::OpenData << AbstractImporter::Feature(0xf0); + CORRADE_COMPARE(out.str(), "Audio::AbstractImporter::Feature::OpenData Audio::AbstractImporter::Feature(0xf0)\n"); +} + +void AbstractImporterTest::debugFeatures() { + std::ostringstream out; + + Debug{&out} << AbstractImporter::Feature::OpenData << AbstractImporter::Features{}; + CORRADE_COMPARE(out.str(), "Audio::AbstractImporter::Feature::OpenData Audio::AbstractImporter::Features{}\n"); } }}}} diff --git a/src/Magnum/Audio/Test/CMakeLists.txt b/src/Magnum/Audio/Test/CMakeLists.txt index 0e934e2ec..9a2598af5 100644 --- a/src/Magnum/Audio/Test/CMakeLists.txt +++ b/src/Magnum/Audio/Test/CMakeLists.txt @@ -34,7 +34,7 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/configure.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/configure.h) corrade_add_test(AudioAbstractImporterTest AbstractImporterTest.cpp - LIBRARIES MagnumAudio + LIBRARIES MagnumAudioTestLib FILES file.bin) target_include_directories(AudioAbstractImporterTest PRIVATE ${CMAKE_CURRENT_BINARY_DIR}) corrade_add_test(AudioBufferFormatTest BufferFormatTest.cpp LIBRARIES MagnumAudio) diff --git a/src/Magnum/Audio/visibility.h b/src/Magnum/Audio/visibility.h index 128def581..7929d564f 100644 --- a/src/Magnum/Audio/visibility.h +++ b/src/Magnum/Audio/visibility.h @@ -31,7 +31,7 @@ #ifndef DOXYGEN_GENERATING_OUTPUT #ifndef MAGNUM_BUILD_STATIC - #ifdef MagnumAudio_EXPORTS + #if defined(MagnumAudio_EXPORTS) || defined(MagnumAudioObjects_EXPORTS) #define MAGNUM_AUDIO_EXPORT CORRADE_VISIBILITY_EXPORT #else #define MAGNUM_AUDIO_EXPORT CORRADE_VISIBILITY_IMPORT