Skip to content

fix(rules): provide coco/gmock_helpers.h with the C++ runtime - #266

Open
GPLeemrijse wants to merge 1 commit into
fix/generate-mocksfrom
feat/cc-testing-gmock
Open

GPLeemrijse wants to merge 1 commit into
fix/generate-mocksfrom
feat/cc-testing-gmock

Conversation

@GPLeemrijse

@GPLeemrijse GPLeemrijse commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

The C++ runtime's coco/gmock_helpers.h (coco::SingleThreadedGMockHelper, coco::MultiThreadedGMockHelper) could not be included by any test: the runtime excluded it because it includes gmock, and the runtime repo's :testing target that held it was referenced nowhere. It is now a textual header of the runtime: never compiled on its own, so the runtime still builds without gmock, and coco_cc_test_library gets it from the library's popili version. GoogleMock stays the library's own gmock dep, as before.

Note: I did go for the textual header in the end, it actually seemed the least bad option:

An earlier version of this PR sent gmock through the toolchain instead (a coco.cc_test_runtime(gmock = ...) tag, per popili version, linked as the toolchain's cc_test_runtime). Review showed that was the wrong place for it: coco_cc_test_library could no longer be built without GoogleMock (gmock = None), a testonly gmock was rejected because a toolchain cannot be testonly, gmock lost its direct dep and so broke layering_check, and a per-version gmock let a pinned test link two googletests. gmock does not depend on the popili version and was already built unpinned; only the header does, and the runtime already carries it per version. So the header goes with the runtime and gmock stays a plain dep, which avoids all four problems and about 400 lines of plumbing.

Tested with the root suite and the smoke (new GMockHelper test) and multi_version (runtimes without the header) e2e modules.

@GPLeemrijse
GPLeemrijse added this pull request to stack #267 November 6, 2026 16:07
@GPLeemrijse
GPLeemrijse force-pushed the feat/cc-testing-gmock branch from 4412b4f to 23bdd3c Compare November 6, 2026 17:07
@GPLeemrijse GPLeemrijse changed the title feat(rules)!: link coco/gmock_helpers.h into coco_cc_test_library, with gmock from coco.cc_testing feat(rules)!: link coco/gmock_helpers.h into coco_cc_test_library, with gmock from coco.cc_test_runtime Nov 6, 2026
@GPLeemrijse
GPLeemrijse force-pushed the feat/cc-testing-gmock branch from 23bdd3c to 5b50892 Compare November 6, 2026 17:15
@GPLeemrijse
GPLeemrijse removed this pull request from stack #267 November 6, 2026 17:39
@GPLeemrijse
GPLeemrijse force-pushed the feat/cc-testing-gmock branch from 5b50892 to 6819a77 Compare November 6, 2026 17:39
@GPLeemrijse
GPLeemrijse added this pull request to stack #270 November 6, 2026 17:40
The runtime repo's :testing target held the header but was referenced nowhere, so no target
provided it. It is now a textual header of :runtime: it includes gmock, which only
coco_cc_test_library links, and as a textual header it is never compiled on its own. It comes
from the library's popili version; gmock stays the library's own dep.
@GPLeemrijse
GPLeemrijse force-pushed the feat/cc-testing-gmock branch from 6819a77 to 5b334fe Compare November 6, 2026 18:23
@GPLeemrijse GPLeemrijse changed the title feat(rules)!: link coco/gmock_helpers.h into coco_cc_test_library, with gmock from coco.cc_test_runtime fix(rules): provide coco/gmock_helpers.h with the C++ runtime Nov 6, 2026
@GPLeemrijse
GPLeemrijse marked this pull request as ready for review November 6, 2026 18:28
@GPLeemrijse
GPLeemrijse requested a review from tomgr November 6, 2026 18:28

@tomgr tomgr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we'll have to find a different approach to this. we should not have it as a textual header and it should have a proper dependency on gmock.

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.

2 participants