Convert double to words model tests to GTest - #111
ninotarantino wants to merge 7 commits into
Conversation
| a resolution of 0.1. This is a common case for flight software telemetry, which tends to pack floating point numbers into | ||
| smaller buffers of known resolution per bit due to limited bandwidth. | ||
|
|
||
| .. graphviz:: |
There was a problem hiding this comment.
Should this content be rendering into a table?
If I try to view file, I get:
There was a problem hiding this comment.
You can download the built documentation from the Upload pages preview stage of the Docs / build job below. Sphinx extensions like graphviz aren't going to render in the PR preview.
There was a problem hiding this comment.
Do you have a presentation template ready to replace this?
There was a problem hiding this comment.
No, this document is something Ramtares needs to own since it's an Antares-specific document. When CML establishes an internal V&V process, if it continues using PowerPoint presentations, a new one will be uploaded.
There was a problem hiding this comment.
What is the strategy for testing build compatibility with Trick?
There was a problem hiding this comment.
At the moment it's the Trickification process but agreed, that won't catch if the library dependency somehow breaks. I'll restore one sim with a notional run.
There was a problem hiding this comment.
Confirming -- Trickification is part of the CI testing?
Can Trickification complete if the library dependency breaks?
There was a problem hiding this comment.
That's right, we build and Trickify CML, then build and run all unit sims. Trickification would still complete even with bad library dependencies since Trick isn't building any of the actual model code. It would choke on SWIG issues. And the libcml build would also complete since that only depends on cmake.
So yeah, I'll need to put one of those unit sims back to test that the library dependencies are valid.
There was a problem hiding this comment.
Would it make sense to have a single "master" sim to build all models to test for Trick compatibility, or keep a small sim in each model?
There was a problem hiding this comment.
Yeah I like that idea! We already need to define an S_source.hh which includes every CML header for Trickification. We could just have a SIM_test_trick_compatibility in utilities which includes that file. Then if it builds, we know the library dependencies in CML are all valid. I'll push something like that next week.
|
coverage: 89.461% (+0.005%) from 89.456% — convert-double-to-words-tests-to-gtest into main |
sylvesterkaczmarek
left a comment
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
This removes the public virtual destructor from ConvertDoubleToUintWords even though the PR is framed as test/docs modernization. That changes the C++ ABI and makes deletion through a base pointer unsafe for downstream subclasses. Unless that API break is intentional, I think the virtual destructor should stay.
Good catch, that's unintentional. Will fix on Monday. |
|
More thoughts on the documentation -- I think the choice to describe the tests in the test-code comments and reference those from the documentation is the wrong way around. These should be comprehensively documented in the model documentation and referenced from the test-code. |
Sure, I can move that over into the docs. |
Added detailed descriptions to each test case in the documentation and fixed the accidental removal of the virtual destructor.
sylvesterkaczmarek
left a comment
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
Rechecked current 7ef567f. The public virtual destructor is restored as virtual ~ConvertDoubleToUintWords() = default;, so the unintended ABI / polymorphic-deletion break I raised is resolved. The current build, static-analysis and test checks are green. No remaining blocker from my review.
This branch has not been deployed
4 participants
Modernize the double to words model tests and convert its documentation to our new Doxygen/Sphinx format.