In this process we also create some other tests, in order to not lose
coverage when focusing on the annotated code.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| clang/unittests/Tooling/Syntax/TreeTestBase.cpp | ||
|---|---|---|
| 197 ↗ | (On Diff #285629) | |
| 199 ↗ | (On Diff #285629) | ASSERT_EQ I think would be better. |
| 200 ↗ | (On Diff #285629) | LLVM style is usually: unsigned i = 0; ... |
| 206 ↗ | (On Diff #285629) | Could you dump the annotated source code substring to make debugging failing tests easier? |
Very nice improvement to tests!
| clang/unittests/Tooling/Syntax/BuildTreeTest.cpp | ||
|---|---|---|
| 1034 | The name of this test should be about declarations, and this test should be in the section of this file that has to do with declarations. | |
| 1285 | The name of this test should be only about typedefs, and not about user-defined literals. The test should be also in the section of this file that is about declarations (down below). | |
| 2744 | I think it should be T var, otherwise T is not used (not using T is valid, but rather unusual). | |
Answering comments
| clang/unittests/Tooling/Syntax/BuildTreeTest.cpp | ||
|---|---|---|
| 477 | A test for this part was created below | |
| clang/unittests/Tooling/Syntax/TreeTestBase.cpp | ||
| 199 ↗ | (On Diff #285629) | ASSERT_EQ is a macro that returns void, so we cannot use it here. However that brings another question. WDYT? I case you agree we can perform this change in another patch. |
| 206 ↗ | (On Diff #285629) | Very good suggestion :) thanks |
Fail on non-matching number of tree dumps and annotations
| clang/unittests/Tooling/Syntax/TreeTestBase.cpp | ||
|---|---|---|
| 199 ↗ | (On Diff #285629) | We need the EXPECT_TRUE at the test site to get information about in which line number we failed |
clang-tidy: error: use of undeclared identifier 'treeDumpEqualOnAnnotations' [clang-diagnostic-error]
not useful