From b0ad5c3ed5341edf03fa10e774cdeddf94c61739 Mon Sep 17 00:00:00 2001 From: Brandon Jones Date: Tue, 14 Jun 2022 10:56:19 +0100 Subject: [PATCH] Change Bazel XML support to depend upon BAZEL_TEST --- docs/configuration.md | 6 ++-- docs/deprecations.md | 9 ++++++ src/catch2/catch_config.cpp | 51 ++++++++++++++++++++++----------- src/catch2/catch_config.hpp | 1 + tests/ExtraTests/CMakeLists.txt | 12 ++++++++ 5 files changed, 60 insertions(+), 19 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index d546334342..0e9a32052b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -98,10 +98,10 @@ is equivalent with the out-of-the-box experience. ## Bazel support -When `CATCH_CONFIG_BAZEL_SUPPORT` is defined, Catch2 will register a `JUnit` -reporter writing to a path pointed by `XML_OUTPUT_FILE` provided by Bazel. +When `CATCH_CONFIG_BAZEL_SUPPORT` is defined or when `BAZEL_TEST=1` (which is set by the Bazel inside of a test environment), +Catch2 will register a `JUnit` reporter writing to a path pointed by `XML_OUTPUT_FILE` provided by Bazel. -> `CATCH_CONFIG_BAZEL_SUPPORT` was [introduced](https://github.com/catchorg/Catch2/pull/2399) in Catch2 3.0.1. +> `CATCH_CONFIG_BAZEL_SUPPORT` was [introduced](https://github.com/catchorg/Catch2/pull/2399) in Catch2 3.0.1. It will be deprecated in Catch2 X.Y.Z. ## C++11 toggles diff --git a/docs/deprecations.md b/docs/deprecations.md index 3319e52b45..2c9bf5517b 100644 --- a/docs/deprecations.md +++ b/docs/deprecations.md @@ -17,6 +17,15 @@ as it can be replaced by `Catch.cmake` that provides the function command line interface instead of parsing C++ code with regular expressions. +### `CATCH_CONFIG_BAZEL_SUPPORT` + +Catch2 supports writing the Bazel JUnit XML output file when it is aware +that is within a bazel testing environment. Originally there was no way +to accurately probe the environment for this information so the flag +`CATCH_CONFIG_BAZEL_SUPPORT` was added. This now deprecated. Bazel has now had a change +where it will export `BAZEL_TEST=1` for purposes like the above. Catch2 +will now instead inspect the environment instead of relying on build configuration. + --- [Home](Readme.md#top) diff --git a/src/catch2/catch_config.cpp b/src/catch2/catch_config.cpp index 670cc4f761..c785c226cc 100644 --- a/src/catch2/catch_config.cpp +++ b/src/catch2/catch_config.cpp @@ -59,27 +59,27 @@ namespace Catch { } ); } -#if defined( CATCH_CONFIG_BAZEL_SUPPORT ) - // Register a JUnit reporter for Bazel. Bazel sets an environment - // variable with the path to XML output. If this file is written to - // during test, Bazel will not generate a default XML output. - // This allows the XML output file to contain higher level of detail - // than what is possible otherwise. + if(hasBazel()){ + // Register a JUnit reporter for Bazel. Bazel sets an environment + // variable with the path to XML output. If this file is written to + // during test, Bazel will not generate a default XML output. + // This allows the XML output file to contain higher level of detail + // than what is possible otherwise. # if defined( _MSC_VER ) - // On Windows getenv throws a warning as there is no input validation, - // since the key is hardcoded, this should not be an issue. -# pragma warning( push ) -# pragma warning( disable : 4996 ) + // On Windows getenv throws a warning as there is no input validation, + // since the key is hardcoded, this should not be an issue. +# pragma warning( push ) +# pragma warning( disable : 4996 ) # endif - const auto bazelOutputFilePtr = std::getenv( "XML_OUTPUT_FILE" ); + const auto bazelOutputFilePtr = std::getenv( "XML_OUTPUT_FILE" ); # if defined( _MSC_VER ) # pragma warning( pop ) # endif - if ( bazelOutputFilePtr != nullptr ) { - m_data.reporterSpecifications.push_back( - { "junit", std::string( bazelOutputFilePtr ), {}, {} } ); - } -#endif + if ( bazelOutputFilePtr != nullptr ) { + m_data.reporterSpecifications.push_back( + { "junit", std::string( bazelOutputFilePtr ), {}, {} } ); + } + } // We now fixup the reporter specs to handle default output spec, @@ -113,6 +113,25 @@ namespace Catch { bool Config::listReporters() const { return m_data.listReporters; } bool Config::listListeners() const { return m_data.listListeners; } + bool Config::hasBazel() const { +#ifdef CATCH_CONFIG_BAZEL_SUPPORT + return true; +#endif + +# if defined( _MSC_VER ) + // On Windows getenv throws a warning as there is no input validation, + // since the switch is hardcoded, this should not be an issue. +# pragma warning( push ) +# pragma warning( disable : 4996 ) +# endif + + return std::getenv("BAZEL_TEST") != nullptr; + +# if defined( _MSC_VER ) +# pragma warning( pop ) +# endif + } + std::vector const& Config::getTestsOrTags() const { return m_data.testsOrTags; } std::vector const& Config::getSectionsToRun() const { return m_data.sectionsToRun; } diff --git a/src/catch2/catch_config.hpp b/src/catch2/catch_config.hpp index 10df4d64ad..cbe24c9cee 100644 --- a/src/catch2/catch_config.hpp +++ b/src/catch2/catch_config.hpp @@ -101,6 +101,7 @@ namespace Catch { bool listTags() const; bool listReporters() const; bool listListeners() const; + bool hasBazel() const; std::vector const& getReporterSpecs() const; std::vector const& diff --git a/tests/ExtraTests/CMakeLists.txt b/tests/ExtraTests/CMakeLists.txt index f33bd7c117..c6f5fec3de 100644 --- a/tests/ExtraTests/CMakeLists.txt +++ b/tests/ExtraTests/CMakeLists.txt @@ -137,6 +137,18 @@ set_tests_properties(CATCH_CONFIG_BAZEL_REPORTER-1 LABELS "uses-python" ) +# We must now test this works without the build flag. +add_executable( BazelReporterNoCatchConfig ${TESTS_DIR}/X30-BazelReporter.cpp ) +target_link_libraries(BazelReporterNoCatchConfig Catch2_buildall_interface) +add_test(NAME NO_CATCH_CONFIG_BAZEL_REPORTER-1 + COMMAND + "${PYTHON_EXECUTABLE}" "${CATCH_DIR}/tests/TestScripts/testBazelReporter.py" $ "${CMAKE_CURRENT_BINARY_DIR}" +) +set_tests_properties(NO_CATCH_CONFIG_BAZEL_REPORTER-1 + PROPERTIES + LABELS "uses-python" + ENVIRONMENT "BAZEL_TEST=1" +) # The default handler on Windows leads to the just-in-time debugger firing, # which makes this test unsuitable for CI and headless runs, as it opens