-
Notifications
You must be signed in to change notification settings - Fork 200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ensure correct handling of mock endpoints #228
Conversation
9d3a05c
to
9840045
Compare
awaiting fix for test breakage in 211bbc9 to rebase and rerun CI. |
9840045
to
f57cc80
Compare
This ensures that, regardless of what has been specified for mocking, only modules with endpoints defined in the config will have servers started for them in a mock system test. This also includes removal of some newer clang-format plugins most of which should eventually be reenabled. Signed-off-by: Sam Stuewe <stuewe@mit.edu>
f57cc80
to
d6674e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
The intended changes look solid, as intended.
A comment that may be beyond the scope of this PR:
It may be valuable if the mock_system
provides some level of self-integrity by ensuring that the input configuration is at least theoretically workable for one of the existing architectures. This would require some documentation and checks with error messages. I may be missing some context, but the self-integrity feature may make this module even more compelling, especially as a c++ module as opposed to a shell utility aimed to serve a similar purpose.
@AlexRamRam note that the mock_system is a testing utility which allows us to mock N components while running one or more “real” components so we can do integration/boundary tests. However, a utility that parses a config and determines if it is even statically-valid for the specified architecture would be an excellent tool to have. (Though, it probably requires us to finalize an answer to #218 before we could reasonably implement one.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
T-ACK. Tests run as expected, and this improves our test suite by allowing endpoints to be mocked properly. Left one comment about the changes to .clang-tidy to consider but think this is good to merge.
This ensures that, regardless of what has been specified for mocking, any module with no endpoints defined in the config will have a server started for it in a mock system test.