Skip to content

C++: Convert qlref tests to inline expectations#22127

Open
owen-mc wants to merge 6 commits into
github:mainfrom
owen-mc:cpp/convert-qlref-inline-expectations
Open

C++: Convert qlref tests to inline expectations#22127
owen-mc wants to merge 6 commits into
github:mainfrom
owen-mc:cpp/convert-qlref-inline-expectations

Conversation

@owen-mc

@owen-mc owen-mc commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions github-actions Bot added the C++ label Jul 5, 2026
@owen-mc owen-mc force-pushed the cpp/convert-qlref-inline-expectations branch 2 times, most recently from 24aac89 to e1753ae Compare July 6, 2026 11:34
@owen-mc

owen-mc commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author
Rerun has been triggered: 2 restarted 🚀

1 similar comment
@owen-mc

owen-mc commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author
Rerun has been triggered: 2 restarted 🚀

@owen-mc owen-mc marked this pull request as ready for review July 7, 2026 08:16
@owen-mc owen-mc requested a review from a team as a code owner July 7, 2026 08:16
Copilot AI review requested due to automatic review settings July 7, 2026 08:16
@owen-mc owen-mc marked this pull request as draft July 7, 2026 08:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@jketema

jketema commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Since I was looking at this already: I see quite some $ Alert // GOOD ... [FALSE POSITIVE] those need a "spurious". Bonus point for also updating // BAD ... [NOT DETECTED] with a missing alert annotation.

@owen-mc owen-mc force-pushed the cpp/convert-qlref-inline-expectations branch from e1753ae to 76d8ae8 Compare July 7, 2026 09:59
@owen-mc owen-mc marked this pull request as ready for review July 7, 2026 12:31
@owen-mc owen-mc requested a review from jketema July 7, 2026 12:32

@jketema jketema left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got up to query-tests/Security, but need a break 😅

Comment thread cpp/ql/test/experimental/query-tests/Security/CWE/CWE-295/CurlSSL.cpp Outdated

@jketema jketema left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final few comments.

I also came across a few lines that now look like:

... // $ ... // $ ...

So two inline expectations. Those probably need to be fixed up too, nut it seems more effective to find those through a grep on the codebase than me flagging them up here.

Comment thread cpp/ql/test/query-tests/Security/CWE/CWE-131/NoSpaceForZeroTerminator/test2.cpp Outdated
Comment thread cpp/ql/test/query-tests/Security/CWE/CWE-131/NoSpaceForZeroTerminator/test2.cpp Outdated
Comment thread cpp/ql/test/query-tests/Security/CWE/CWE-807/semmle/TaintedCondition/test.cpp Outdated
owen-mc added 2 commits July 7, 2026 20:48
Note that when a line is marked `// BAD [NOT DETECTED]` but actually has
an alert, I assume that the `[NOT DETECTED]` is outdated and should be
deleted.
@owen-mc owen-mc force-pushed the cpp/convert-qlref-inline-expectations branch from caaa912 to 19745e1 Compare July 7, 2026 19:48
@owen-mc owen-mc requested a review from jketema July 7, 2026 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants