Skip to content

Add DISCOVERY_ENVIRONMENT option to catch_discover_tests - #3200

Open
itz-puneet wants to merge 2 commits into
catchorg:develfrom
itz-puneet:discovery-environment
Open

Add DISCOVERY_ENVIRONMENT option to catch_discover_tests#3200
itz-puneet wants to merge 2 commits into
catchorg:develfrom
itz-puneet:discovery-environment

Conversation

@itz-puneet

Copy link
Copy Markdown

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.

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

Reported cases include a missing shared library search path, a Qt
platform plugin that has to be set to `offscreen` on a headless machine,
and an instrumentation library whose extra output corrupts the JSON
listing.

`DL_PATHS` already solves this for the specific case of dynamic linker
search paths. This adds `DISCOVERY_ENVIRONMENT`, which does the same for
arbitrary `<var>=<value>` pairs.

The variables are applied only for the duration of discovery, and any
previous values are restored afterwards. That restore matters in
PRE_TEST mode, where discovery runs inside the CTest process, and
without it the variables would leak into the environment of the tests -
which is explicitly not what the option promises.

Closes catchorg#1810
Copilot AI lite review requested due to automatic review settings September 6, 2026 18:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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
File Description
extras/Catch.cmake Adds the public DISCOVERY_ENVIRONMENT option and forwards it into the discovery script for POST_BUILD and PRE_TEST modes.
extras/CatchAddTests.cmake Applies TEST_DISCOVERY_ENVIRONMENT during discovery and restores environment afterward.
docs/cmake-integration.md Documents the new option, its scope (discovery-only), and provides usage examples.
tests/CMakeLists.txt Registers the new CMake helper test.
tests/TestScripts/DiscoverTests/TestDiscoveryEnvironment.cmake Adds a unit test verifying propagation and non-leak/restoration semantics.
tests/TestScripts/DiscoverTests/FakeListingExecutable.cmake Adds 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.

Comment thread extras/CatchAddTests.cmake
Comment thread tests/TestScripts/DiscoverTests/TestDiscoveryEnvironment.cmake
Comment thread tests/TestScripts/DiscoverTests/TestDiscoveryEnvironment.cmake
A repeated entry such as `FOO=a;FOO=b` stashed the pre-discovery value on
every iteration. By the second one `FOO` had already been modified, so the
stash held `a` rather than the original value, and the restore put back `a`
instead of what was there before discovery. The value is now stashed only
the first time a name is seen, which leaves last-one-wins behaviour during
discovery unchanged.

The unit test asserted on a variable it inherited from the caller, so it
could fail spuriously for anyone who happened to have that variable set,
and its cleanup unset the variable unconditionally, clobbering a value the
caller may have wanted. It now stashes and clears the variable up front and
restores it at the end.

Also adds a regression case covering the repeated-entry behaviour.
@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ 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     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

catch_discover_tests doesn't use the same environment to discover tests through TEST_EXECUTABLE

2 participants