Closed JakeHillion closed 6 months ago
Attention: Patch coverage is 0%
with 73 lines
in your changes are missing coverage. Please review.
Project coverage is 60.85%. Comparing base (
a4723fb
) to head (41e9f1c
).
Files | Patch % | Lines |
---|---|---|
oi/type_graph/ClangTypeParserTest.cpp | 0.00% | 73 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
tests: add ClangTypeParserTest
Currently there is no testing for ClangTypeParser even though it's used in production. This is because adding integration tests is very hard: they require testing the build time behaviour at runtime, or else they'd be build failures intead of test failures. There's a PR available for integration tests but it's incomplete.
In contrast ClangTypeParser can be sort of unit tested. This follows the structure of
test/test_drgn_parser.cpp
with some differences. There is a tonne of boilerplate for setting up the Clang tool, and this set of testing operates on type names instead of OID functions. The new tests are also incredibly slow as they compile the entireintegration_test_target.cpp
(which is huge) for every test case. I don't think this is avoidable without compromising the separation of the tests somewhat due to the way Clang tooling forces the code to be structured.Currently I can't run these tests locally on a Meta devserver due to some weirdness with the internal build and the
compile_commands.json
file. They run in the CI and on any other open source machine though so I'm happy to merge it - it's still useful. I'm going to close the PR to change the devserver build given I'll be unable to follow up if it ends up being bad.Test plan: