-
Notifications
You must be signed in to change notification settings - Fork 749
fix(amazonq): ignored lines should not show up in /review scan issues #6659
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
Conversation
|
| const aggregatedCodeScanIssue: AggregatedCodeScanIssue = { | ||
| filePath: filePath, | ||
| issues: issues.map((issue) => mapRawToCodeScanIssue(issue, editor, jobId, scope)), | ||
| issues: issues.map((issue) => mapRawToCodeScanIssue(issue, jobId, scope, document)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: you can make this change even smaller by keeping the position of the editor/document argument the same
| }) | ||
|
|
||
| function getWorkspaceFolder(): string { | ||
| return path.join(__dirname, '../../../../../../core/dist/src/testFixtures/workspaceFolder') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the src path instead of the dist path?
| nextToken: nextToken ? 'nextToken' : undefined, | ||
| }) | ||
|
|
||
| function getWorkspaceFolder(): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use something like vscode.workspace.workspaceFolders[0].uri instead ? Aren't we already in the workspace folder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
packages/amazonq/test/unit/codewhisperer/service/securityScanHandler.test.ts
Outdated
Show resolved
Hide resolved
…andler.test.ts Co-authored-by: Tai Lai <[email protected]>
Problem
When running a new
/reviewscan, Q fails to respect previously added ignore line, causing ignored issues to reappear in subsequent reviews.Solution
Fixed incorrect filter logic on ignoring single line issue
Old unit tests for
listScanResultswill not work after this change. So in addition, I improved all ofsecurityScanHandler's unit tests to not use any file stubs to better simulate real environment.feature/xbranches will not be squash-merged at release time.