#3188

Add flowId to TeamCity reporter's service messages

Openmmakhalafwants to merge intofrom

Description

catch_discover_tests() (Catch2's own CMake integration, extras/Catch.cmake) registers one CTest test per Catch2 test case, and each one runs the test executable as its own separate process, filtered down to that single case. When CTest runs many of those processes concurrently (ctest -j N) with --reporter teamcity, every process writes its own ##teamcity[testSuiteStarted ...] / testStarted / testFinished / testSuiteFinished sequence into what TeamCity's build log ultimately sees as one combined stream.

TeamCity's service-message protocol has a flowId attribute specifically for this: it lets TeamCity tell messages coming from different concurrent flows (e.g. different processes) apart. A message with no flowId is treated as belonging to a single implicit default flow. Without it, if two concurrent test processes' testSuiteStarted/testSuiteFinished messages interleave even slightly - which they will, under real parallel execution - TeamCity attributes a later suite's testSuiteStarted as nested inside an earlier, not-yet-closed suite in the same default flow. The result is garbled, concatenated test names in TeamCity's UI, e.g.:

Graphics_Test.exe: Vd_Test.exe: Mc_Test.exe: End Mill (1/16")/Ball Nose (1/8"), Model Boundary, Offset/Climb Strategy

Pass/fail status is still individually correct for each process - this is a display/attribution bug, not a correctness bug - but it makes results unreadable, and breaks anything keyed on a stable test name (mute rules, flaky-test history, etc). This isn't specific to any one project's CMake setup; it's a general consequence of running Catch2's TeamCity reporter under any one-process-per-test-case parallel test runner.

This PR tags all six ##teamcity[...] message sites emitted by TeamCityReporter (testSuiteStarted, testSuiteFinished, testStarted, testFinished, testStdOut/testStdErr, and testFailed/testIgnored) with flowId='<pid>', using a small process-id helper guarded by the existing CATCH_PLATFORM_WINDOWS macro (the pattern already used for platform-specific code elsewhere, e.g. catch_debugger.cpp).

A couple of design choices worth calling out for review:

  • Always-on, not opt-in. TeamCity's own docs describe flowId as safe to add unconditionally - consumers that don't care about flows ignore it. This seemed like the simplest, most defensible default rather than gating it behind a new reporter option, but I'm happy to make it configurable if that's preferred.
  • Process id as the flow key. The pid only needs to be unique among processes whose output could plausibly interleave within the same build step - it doesn't need to be globally unique across machines or time. A pid is stable for the lifetime of one process and satisfies that.

Tested: rebuilt with the all-tests preset (examples, extra tests, benchmarks, CMake config tests) and ran the full suite via ctest, all passing. Updated the two TeamCity ApprovalTests baselines to include the new attribute, and added a normalizer (teamcityFlowIdParser) to tools/scripts/approvalTests.py so the non-deterministic pid is replaced with a {pid} placeholder before diffing, the same way {duration} is already handled. Manually confirmed with two concurrent processes emitting to the same terminal that each gets a distinct, stable flowId.

GitHub Issues

Not tied to an existing issue - I searched the tracker for prior reports (mangled/nested/duplicate TeamCity output, flowId, parallel ctest) and didn't find one, so I'm submitting the fix directly. Happy to open an issue first if that's preferred process.