fix(lifecycle_client): link lifecycle_mock against lifecyclemanager t… - #337
fix(lifecycle_client): link lifecycle_mock against lifecyclemanager t…#337Chahult wants to merge 4 commits into
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run --lockfile_mode=error //:license-checkStatus: Click to expand output |
|
@Chahult some checks are failing,are you planning to adapt or delete the PR? |
bd15106 to
73c9454
Compare
73c9454 to
2da00ec
Compare
2da00ec to
4e403ba
Compare
|
The created documentation from the pull request is available at: docu-html |
4e403ba to
9ae0e52
Compare
|
@NicolasFussberger Could you please review |
Adapt the changes for config management |
9ae0e52 to
2de1db3
Compare
lifecycle_mock provides a mock of LifeCycleManager which inherits from the concrete class. Without :lifecyclemanager in its deps the vtable symbol is undefined when building lifecycle_mock as a shared library (--no-allow-shlib-undefined). Also guard all ApplicationContext callbacks against null to prevent std::bad_function_call when a global ApplicationContext is constructed before any ApplicationContextMock instance exists.
2de1db3 to
5ddb37e
Compare
|
@Chahult In the lifecycle repo we have a bit the problem that these mocks were contributed but we have no tests for them at the moment and its also not really obvious how to use them. Would it be possible for you to contribute some tests based on your usage of these mocks? E.g. for some example application we have in the examples folder? |
Sure, Done — added examples/cpp_lifecycle_app/lifecycle_mocks_UT.cpp with tests that mirror how config_management uses the mocks (argument injection via ApplicationContextMock, lifecycle flow via LifeCycleManagerMock). These also serve as usage documentation for future contributors. |
80d7948 to
f619102
Compare
279936a to
b5ac84b
Compare
b5ac84b to
0df76f4
Compare
| visibility = ["//score/launch_manager:__subpackages__"], | ||
| deps = [ | ||
| ":applicationcontext_mock", | ||
| ":lifecyclemanager", |
There was a problem hiding this comment.
Is it really correct to add the lifecylemanager here?
Now this target contains both, the lifecyclemanagermock.cpp and the actual lifecyclemanager implementation
There was a problem hiding this comment.
Yes, it's intentional. LifeCycleManagerMock inherits from the concrete LifeCycleManager class. Without :lifecyclemanager in its deps, the vtable symbols for the base class are undefined when building lifecycle_mock as a shared library under --no-allow-shlib-undefined.
Adding :lifecyclemanager provides the vtable — this is the minimum fix to make the linker happy. The mock .cpp itself remains unchanged; it just needs the base class compiled in.
| actual = "//score/launch_manager/src/lifecycle_client:lifecycle_mock", | ||
| testonly = True, | ||
| deps = [ | ||
| "//score/launch_manager/src/lifecycle_client:applicationcontext_mock", |
There was a problem hiding this comment.
I think applicationcontext mock is already part of //score/launch_manager/src/lifecycle_client:lifecycle_mock
There was a problem hiding this comment.
You're right , lifecycle_mock already depends on :applicationcontext_mock transitively. Removed the explicit dep from lifecycle_mock_cc
- Move lifecycle_mocks_UT from examples/ into lifecycle_client/src/ (self-contained test, not coupled to the example app) - Remove lifecycle_test_mocks aggregator target; adapt lifecycle_mock_cc in //score/launch_manager to aggregate all three mock targets instead - Revert accidental visibility widening on applicationcontext, application, and runapplication back to //score/launch_manager:__subpackages__
0df76f4 to
e179e93
Compare
…o resolve vtable
lifecycle_mock provides a mock implementation of LifeCycleManager, which inherits from the concrete class defined in lifecyclemanager.cpp. Without linking against the :lifecyclemanager target, the vtable symbol is undefined when building lifecycle_mock as a shared library (--no-allow-shlib-undefined).
Added :lifecyclemanager to lifecycle_mock's deps to provide the vtable.