Supplement 'query-type: graph' with actual query metadata #17823
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jketema
reviewed
Oct 22, 2024
There was a problem hiding this comment.
It looks like the auto-formatter is not quite happy, otherwise this LGTM. You might want to check that none of our internal tests use query-type. And if you're really paranoid you might also want to check https://github.com/github/codeql-coding-standards.
A number of CPP library tests contain `// query-type: graph` annotations that make the test driver compare the output from the test query in a special mode. (This feature is not used by other languages). It's somewhat awkward in the implementation of `codeql test run` that this annotation is not an ordinary item of query metadata -- essentially it means that _every_ test query has to be opened and read an extra time to look for this annotation. I'd like to move towards using ordinary query metadata for this, since the QL compiler already parses it anyway. For the time being, give the annotation in both old and new syntaxes, until a CLI that recognizes both has been released.
hmakholm
force-pushed
the
hmakholm/pr/graph-equivalence-test
branch
from
f233a5a to
3d8d340
Compare
jketema
approved these changes
Oct 22, 2024
|
Ran the autoformatter. I grepped through all The only files that even contain the string |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A number of CPP library tests contain
// query-type: graphannotations that make the test driver compare the output from the test query in a special mode. (This feature is not used by other languages).It's somewhat awkward in the implementation of
codeql test runthat this annotation is not an ordinary item of query metadata -- essentially it means that every test query has to be opened and read an extra time to look for this annotation. I'd like to move towards using ordinary query metadata for this, since the QL compiler already parses it anyway.(It seems unlikely that there exist any test outside this repository that uses
// query-type: graph).For the time being, give the annotation in both old and new syntaxes, until a CLI that recognizes both has been released.