Skip to content
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

Add a unit test suite for HeuristicResolver #2154

Open
Tracked by #121310
HighCommander4 opened this issue Sep 23, 2024 · 3 comments · May be fixed by llvm/llvm-project#121313
Open
Tracked by #121310

Add a unit test suite for HeuristicResolver #2154

HighCommander4 opened this issue Sep 23, 2024 · 3 comments · May be fixed by llvm/llvm-project#121313
Assignees

Comments

@HighCommander4
Copy link

HighCommander4 commented Sep 23, 2024

In preparation for upstreaming HeuristicResolver to the clang libraries as discussed at #1662, I'd like to add a unit test suite exercising the interface of the class directly, which could then be upstreamed together with it.

The class does have test coverage in clangd, but through unit tests that exercise higher layers of clangd functionality and thus have dependencies on other components in clangd and are not candidates for upstreaming to clang.

@HighCommander4 HighCommander4 self-assigned this Sep 23, 2024
@HighCommander4
Copy link
Author

HighCommander4 commented Sep 23, 2024

One thing that's annoying here is that, in addition to (obviously) not being able to use high-level clangd functionality like locateSymbolAt(), tests meant to be upstreamable also don't get to use things like SelectionTree (or indeed, TestTU or ParsedAST, though those are easier to replace than SelectionTree).

@HighCommander4
Copy link
Author

HighCommander4 commented Dec 16, 2024

tests meant to be upstreamable also don't get to use things like SelectionTree

It looks like AST matchers are a suitable upstream replacement for SelectionTree for the purpose of selecting AST nodes as inputs for a piece of code you want to unit-test.

@HighCommander4
Copy link
Author

Proposed fix: llvm/llvm-project#121313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant