From 27769efa07139a7f27d9f024069bff4168f6cbe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 21 Dec 2022 13:30:54 +0100 Subject: [PATCH] GL: improve testing of Shader and AbstractShaderProgram error cases. Had this in a stash for quite a while, not sure why I didn't commit that already. --- src/Magnum/GL/CMakeLists.txt | 4 +-- .../GL/Test/AbstractShaderProgramGLTest.cpp | 10 ++++++ src/Magnum/GL/Test/CMakeLists.txt | 2 +- src/Magnum/GL/Test/ShaderGLTest.cpp | 31 +++++++++++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/Magnum/GL/CMakeLists.txt b/src/Magnum/GL/CMakeLists.txt index cbde1aa2d..85583bdd7 100644 --- a/src/Magnum/GL/CMakeLists.txt +++ b/src/Magnum/GL/CMakeLists.txt @@ -37,7 +37,6 @@ set(MagnumGL_SRCS OpenGL.cpp Renderbuffer.cpp Renderer.cpp - Shader.cpp Texture.cpp TextureFormat.cpp TimeQuery.cpp @@ -65,7 +64,8 @@ set(MagnumGL_GracefulAssert_SRCS Mesh.cpp MeshView.cpp PixelFormat.cpp - Sampler.cpp) + Sampler.cpp + Shader.cpp) set(MagnumGL_HEADERS AbstractFramebuffer.h diff --git a/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp b/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp index 882f937bf..54cdcbc22 100644 --- a/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp +++ b/src/Magnum/GL/Test/AbstractShaderProgramGLTest.cpp @@ -523,8 +523,14 @@ void AbstractShaderProgramGLTest::linkFailure() { Utility::System::sleep(200); CORRADE_VERIFY(program.isLinkFinished()); + + /* There's a driver-specific message after */ CORRADE_COMPARE_AS(out.str(), "GL::AbstractShaderProgram::link(): linking failed with the following message:", TestSuite::Compare::StringHasPrefix); + /* No stray \0 should be anywhere */ + CORRADE_COMPARE_AS(out.str(), "\0"_s, TestSuite::Compare::StringNotContains); + /* The message should end with a newline */ + CORRADE_COMPARE_AS(out.str(), "\n"_s, TestSuite::Compare::StringHasSuffix); } void AbstractShaderProgramGLTest::linkFailureAsync() { @@ -573,6 +579,10 @@ void AbstractShaderProgramGLTest::linkFailureAsync() { CORRADE_VERIFY(program.isLinkFinished()); CORRADE_COMPARE_AS(out.str(), "GL::AbstractShaderProgram::link(): linking failed with the following message:", TestSuite::Compare::StringHasPrefix); + + /* Not testing presence of \0 etc., as that's tested well enough in + linkFailure() above already and both cases use the same error printing + code path */ } void AbstractShaderProgramGLTest::linkFailureAsyncShaderList() { diff --git a/src/Magnum/GL/Test/CMakeLists.txt b/src/Magnum/GL/Test/CMakeLists.txt index 29ba9d095..d950967bd 100644 --- a/src/Magnum/GL/Test/CMakeLists.txt +++ b/src/Magnum/GL/Test/CMakeLists.txt @@ -141,7 +141,7 @@ if(MAGNUM_BUILD_GL_TESTS) endif() corrade_add_test(GLShaderGLTest ShaderGLTest.cpp - LIBRARIES MagnumOpenGLTester + LIBRARIES MagnumOpenGLTesterTestLib FILES ShaderGLTestFiles/shader.glsl) target_include_directories(GLShaderGLTest PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/$) diff --git a/src/Magnum/GL/Test/ShaderGLTest.cpp b/src/Magnum/GL/Test/ShaderGLTest.cpp index 03fda08c5..d0fba6dc9 100644 --- a/src/Magnum/GL/Test/ShaderGLTest.cpp +++ b/src/Magnum/GL/Test/ShaderGLTest.cpp @@ -55,6 +55,7 @@ struct ShaderGLTest: OpenGLTester { void addSource(); void addSourceNoVersion(); void addFile(); + void addFileNonexistent(); void compile(); void compileAsync(); void compileFailure(); @@ -98,6 +99,7 @@ ShaderGLTest::ShaderGLTest() { &ShaderGLTest::addSource, &ShaderGLTest::addSourceNoVersion, &ShaderGLTest::addFile, + &ShaderGLTest::addFileNonexistent, &ShaderGLTest::compile, &ShaderGLTest::compileAsync}); @@ -314,6 +316,24 @@ void ShaderGLTest::addFile() { #endif } +void ShaderGLTest::addFileNonexistent() { + CORRADE_SKIP_IF_NO_ASSERT(); + + #ifndef MAGNUM_TARGET_GLES + Shader shader(Version::GL210, Shader::Type::Fragment); + #else + Shader shader(Version::GLES200, Shader::Type::Fragment); + #endif + + std::ostringstream out; + Error redirectError{&out}; + shader.addFile("nonexistent"); + /* There's an error message from Path::read() before */ + CORRADE_COMPARE_AS(out.str(), + "\nGL::Shader::addFile(): can't read nonexistent\n", + TestSuite::Compare::StringHasSuffix); +} + void ShaderGLTest::compile() { #ifndef MAGNUM_TARGET_GLES constexpr Version v = @@ -379,6 +399,8 @@ void ShaderGLTest::compileFailure() { CORRADE_VERIFY(!shader.compile()); } CORRADE_VERIFY(shader.isCompileFinished()); + + /* There's a driver-specific message after */ CORRADE_COMPARE_AS(out.str(), "GL::Shader::compile(): compilation of vertex shader failed with the following message:", TestSuite::Compare::StringHasPrefix); @@ -397,6 +419,11 @@ void ShaderGLTest::compileFailure() { CORRADE_COMPARE_AS(out.str(), "175", TestSuite::Compare::StringNotContains); CORRADE_COMPARE_AS(out.str(), "176", TestSuite::Compare::StringContains); CORRADE_COMPARE_AS(out.str(), "177", TestSuite::Compare::StringNotContains); + + /* No stray \0 should be anywhere */ + CORRADE_COMPARE_AS(out.str(), "\0"_s, TestSuite::Compare::StringNotContains); + /* The message should end with a newline */ + CORRADE_COMPARE_AS(out.str(), "\n"_s, TestSuite::Compare::StringHasSuffix); } void ShaderGLTest::compileFailureAsync() { @@ -435,6 +462,10 @@ void ShaderGLTest::compileFailureAsync() { CORRADE_VERIFY(shader.isCompileFinished()); CORRADE_COMPARE_AS(out.str(), "GL::Shader::compile(): compilation of fragment shader failed with the following message:", TestSuite::Compare::StringHasPrefix); + + /* Not testing presence of \0 etc., as that's tested well enough in + compileFailure() above already and both cases use the same error printing + code path */ } void ShaderGLTest::compileUtf8() {