#3200

Add DISCOVERY_ENVIRONMENT option to catch_discover_tests

Openitz-puneetwants to merge intofrom

Closes #1810

Description

Test discovery runs the test executable in a separate process, which does not inherit anything set through PROPERTIES ENVIRONMENT — those are applied by CTest to the tests themselves, not to the listing run. An executable that needs a particular environment just to start up and list its test cases therefore fails at discovery time, even though the tests would run fine.

The cases reported in #1810 are a missing shared library search path, a Qt platform plugin needing offscreen on a headless machine, and an instrumentation library whose extra output corrupts the JSON listing.

DL_PATHS already solves this for dynamic linker search paths specifically, and its documented behaviour ("set when retrieving the list of test cases ... and when the tests are executed") is exactly the semantic people want here. This adds DISCOVERY_ENVIRONMENT for arbitrary <var>=<value> pairs.

catch_discover_tests(tests
                     DISCOVERY_ENVIRONMENT QT_QPA_PLATFORM=offscreen)

Scope decision

The option applies to discovery only, not to test execution. Every case in the issue is discovery-side — the tests themselves already ran correctly via PROPERTIES ENVIRONMENT. Making it discovery-scoped keeps the naming unambiguous and changes no existing behaviour. Happy to widen it to cover both if you would rather it mirror DL_PATHS exactly.

Restoring the previous environment

The variables are applied only for the duration of discovery and previous values are restored afterwards, including unsetting variables that were not previously defined. This matters in PRE_TEST mode, where catch_discover_tests_impl runs inside the CTest process — without the restore the variables would leak into the tests' environment, which is the opposite of what the option promises.

The saved values are kept in per-variable CMake variables rather than parallel lists, because an environment variable can be unset or hold an empty string and CMake lists cannot represent those two cases distinctly.

Testing

Added CMakeHelper::DiscoveryEnvironment, a pure-CMake unit test alongside the existing TestPrepareCommandFragment / TestDecomposeJsonArray ones. It drives catch_discover_tests_impl against a fake listing executable that reports a sentinel environment variable back through the test name, and asserts:

  • the sentinel is unset when the option is not given
  • the value reaches the listing process when it is
  • the variable does not leak past discovery
  • a pre-existing value is overridden during discovery
  • a pre-existing value is restored afterwards

The fake executable is a cmake -P script rather than a shell script, so the test runs unchanged on Windows.

All five assertions pass locally, and the two existing CMake helper tests still pass. Note that I ran these via cmake -P directly; I have not built the full test suite, so the wider CI run is the first check of the tests/CMakeLists.txt registration.

Worth flagging: DL_PATHS and DL_FRAMEWORK_PATHS are documented in the Catch.cmake header comment but missing from the option list in docs/cmake-integration.md. I only added the new option there rather than fix that separately — happy to do so in another PR if useful.

Activity

8
copilot-pull-request-reviewer[bot]reviewed1mo ago
copilot-pull-request-reviewer[bot]1mo ago

🟡 Changes recommended

The DISCOVERY_ENVIRONMENT save/restore logic can restore incorrect values when the same variable is specified multiple times, and the new helper test currently depends on the caller’s ambient environment (potential flakiness).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a CMake-facing way to inject environment variables specifically for Catch2 test discovery (the --list-tests run), addressing cases where the test executable needs a special environment just to start and emit JSON.

Changes:

  • Adds DISCOVERY_ENVIRONMENT <var>=<value>... to catch_discover_tests and threads it through to catch_discover_tests_impl.
  • Implements temporary environment application + restoration around the discovery run in CatchAddTests.cmake.
  • Adds a pure-CMake helper test (and a fake listing “executable”) plus documentation updates for the new option.
File summaries
FileDescription
extras/Catch.cmakeAdds the public DISCOVERY_ENVIRONMENT option and forwards it into the discovery script for POST_BUILD and PRE_TEST modes.
extras/CatchAddTests.cmakeApplies TEST_DISCOVERY_ENVIRONMENT during discovery and restores environment afterward.
docs/cmake-integration.mdDocuments the new option, its scope (discovery-only), and provides usage examples.
tests/CMakeLists.txtRegisters the new CMake helper test.
tests/TestScripts/DiscoverTests/TestDiscoveryEnvironment.cmakeAdds a unit test verifying propagation and non-leak/restoration semantics.
tests/TestScripts/DiscoverTests/FakeListingExecutable.cmakeAdds a fake listing script used by the new helper test to reflect environment state in the discovered test name.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

itz-puneetreviewed1mo ago
itz-puneetreviewed1mo ago
itz-puneetreviewed1mo ago
codecov[bot]1mo ago

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 91.21%. Comparing base (897d804) to head (7732946).

Additional details and impacted files
@@            Coverage Diff             @@
##            devel    #3200      +/-   ##
==========================================
+ Coverage   91.17%   91.21%   +0.03%     
==========================================
  Files         206      206              
  Lines        9031     9031              
==========================================
+ Hits         8234     8237       +3     
+ Misses        797      794       -3     
:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
itz-puneet24d ago

@horenmar when you get a chance, could you take a look at this one? It adds DISCOVERY_ENVIRONMENT to catch_discover_tests for #1810, so an executable that needs particular environment variables just to list its tests can be discovered.

CI is green (AppVeyor + codecov), and the Copilot review comments are addressed. The option is threaded through all four call sites, and there's a pure-CMake test (CMakeHelper::DiscoveryEnvironment) that uses cmake -P as the fake executable so it runs on Windows too.