Skip to content

Commit e913055

Browse files
feat(amazonq): add unit tests for parseFindingsMessage
1 parent 3725505 commit e913055

File tree

2 files changed

+279
-62
lines changed

2 files changed

+279
-62
lines changed

plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/webview/BrowserConnector.kt

Lines changed: 66 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -641,74 +641,78 @@ class BrowserConnector(
641641
}
642642
}
643643

644-
private fun parseFindingsMessages(messagesMap: Map<*, *>) {
645-
val additionalMessages = messagesMap["additionalMessages"] as? MutableList<Map<String, Any>>
646-
val findingsMessages = additionalMessages?.filter { message ->
647-
message["messageId"] != null && (message["messageId"] as String).endsWith(CODE_REVIEW_FINDINGS_SUFFIX) ||
648-
(message["messageId"] as String).endsWith(DISPLAY_FINDINGS_SUFFIX)
649-
}
650-
val scannedFiles = mutableListOf<VirtualFile>()
651-
if (findingsMessages != null) {
652-
for (findingsMessage in findingsMessages) {
653-
additionalMessages.remove(findingsMessage)
654-
val gson = Gson()
655-
val jsonFindings = gson.fromJson(findingsMessage["body"] as String, List::class.java)
656-
val mappedFindings = mutableListOf<CodeWhispererCodeScanIssue>()
657-
for (aggregatedIssueUnformatted in jsonFindings) {
658-
val aggregatedIssue = gson.fromJson(gson.toJson(aggregatedIssueUnformatted), AggregatedCodeScanIssue::class.java)
659-
val file = LocalFileSystem.getInstance().findFileByIoFile(
660-
Path.of(aggregatedIssue.filePath).toFile()
661-
)
662-
if (file?.isDirectory == false) {
663-
scannedFiles.add(file)
664-
runReadAction {
665-
FileDocumentManager.getInstance().getDocument(file)
666-
}?.let { document ->
667-
for (issue in aggregatedIssue.issues) {
668-
val endLineInDocument = minOf(maxOf(0, issue.endLine - 1), document.lineCount - 1)
669-
val endCol = document.getLineEndOffset(endLineInDocument) - document.getLineStartOffset(endLineInDocument) + 1
670-
val isIssueIgnored = CodeWhispererCodeScanManager.getInstance(project)
671-
.isIgnoredIssue(issue.title, document, file, issue.startLine - 1)
672-
if (isIssueIgnored) {
673-
continue
644+
fun parseFindingsMessages(messagesMap: Map<*, *>) {
645+
try {
646+
val additionalMessages = messagesMap["additionalMessages"] as? MutableList<Map<String, Any>>
647+
val findingsMessages = additionalMessages?.filter { message ->
648+
message["messageId"] != null && (message["messageId"] as String).endsWith(CODE_REVIEW_FINDINGS_SUFFIX) ||
649+
(message["messageId"] as String).endsWith(DISPLAY_FINDINGS_SUFFIX)
650+
}
651+
val scannedFiles = mutableListOf<VirtualFile>()
652+
if (findingsMessages != null) {
653+
for (findingsMessage in findingsMessages) {
654+
additionalMessages.remove(findingsMessage)
655+
val gson = Gson()
656+
val jsonFindings = gson.fromJson(findingsMessage["body"] as String, List::class.java)
657+
val mappedFindings = mutableListOf<CodeWhispererCodeScanIssue>()
658+
for (aggregatedIssueUnformatted in jsonFindings) {
659+
val aggregatedIssue = gson.fromJson(gson.toJson(aggregatedIssueUnformatted), AggregatedCodeScanIssue::class.java)
660+
val file = LocalFileSystem.getInstance().findFileByIoFile(
661+
Path.of(aggregatedIssue.filePath).toFile()
662+
)
663+
if (file?.isDirectory == false) {
664+
scannedFiles.add(file)
665+
runReadAction {
666+
FileDocumentManager.getInstance().getDocument(file)
667+
}?.let { document ->
668+
for (issue in aggregatedIssue.issues) {
669+
val endLineInDocument = minOf(maxOf(0, issue.endLine - 1), document.lineCount - 1)
670+
val endCol = document.getLineEndOffset(endLineInDocument) - document.getLineStartOffset(endLineInDocument) + 1
671+
val isIssueIgnored = CodeWhispererCodeScanManager.getInstance(project)
672+
.isIgnoredIssue(issue.title, document, file, issue.startLine - 1)
673+
if (isIssueIgnored) {
674+
continue
675+
}
676+
mappedFindings.add(
677+
CodeWhispererCodeScanIssue(
678+
startLine = issue.startLine,
679+
startCol = 1,
680+
endLine = issue.endLine,
681+
endCol = endCol,
682+
file = file,
683+
project = project,
684+
title = issue.title,
685+
description = issue.description,
686+
detectorId = issue.detectorId,
687+
detectorName = issue.detectorName,
688+
findingId = issue.findingId,
689+
ruleId = issue.ruleId,
690+
relatedVulnerabilities = issue.relatedVulnerabilities,
691+
severity = issue.severity,
692+
recommendation = issue.recommendation,
693+
suggestedFixes = issue.suggestedFixes,
694+
codeSnippet = emptyList(),
695+
isVisible = true,
696+
autoDetected = issue.autoDetected,
697+
scanJobId = issue.scanJobId,
698+
),
699+
)
674700
}
675-
mappedFindings.add(
676-
CodeWhispererCodeScanIssue(
677-
startLine = issue.startLine,
678-
startCol = 1,
679-
endLine = issue.endLine,
680-
endCol = endCol,
681-
file = file,
682-
project = project,
683-
title = issue.title,
684-
description = issue.description,
685-
detectorId = issue.detectorId,
686-
detectorName = issue.detectorName,
687-
findingId = issue.findingId,
688-
ruleId = issue.ruleId,
689-
relatedVulnerabilities = issue.relatedVulnerabilities,
690-
severity = issue.severity,
691-
recommendation = issue.recommendation,
692-
suggestedFixes = issue.suggestedFixes,
693-
codeSnippet = emptyList(),
694-
isVisible = true,
695-
autoDetected = issue.autoDetected,
696-
scanJobId = issue.scanJobId,
697-
),
698-
)
699701
}
700702
}
701703
}
702-
}
703704

704-
CodeWhispererCodeScanManager.getInstance(project)
705-
.addOnDemandIssues(
706-
mappedFindings,
707-
scannedFiles,
708-
CodeWhispererConstants.CodeAnalysisScope.AGENTIC
709-
)
710-
CodeWhispererCodeScanManager.getInstance(project).showCodeScanUI()
705+
CodeWhispererCodeScanManager.getInstance(project)
706+
.addOnDemandIssues(
707+
mappedFindings,
708+
scannedFiles,
709+
CodeWhispererConstants.CodeAnalysisScope.AGENTIC
710+
)
711+
CodeWhispererCodeScanManager.getInstance(project).showCodeScanUI()
712+
}
711713
}
714+
} catch (e: Exception) {
715+
LOG.error { "Failed to parse findings message $e" }
712716
}
713717
}
714718

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package software.aws.toolkits.jetbrains.services.amazonq.webview
5+
6+
import com.google.gson.Gson
7+
import com.intellij.openapi.editor.Document
8+
import com.intellij.openapi.fileEditor.FileDocumentManager
9+
import com.intellij.openapi.vfs.LocalFileSystem
10+
import com.intellij.openapi.vfs.VirtualFile
11+
import com.intellij.testFramework.fixtures.CodeInsightTestFixture
12+
import com.intellij.testFramework.replaceService
13+
import org.junit.Before
14+
import org.junit.Test
15+
import org.mockito.Mockito.mockStatic
16+
import org.mockito.kotlin.any
17+
import org.mockito.kotlin.argumentCaptor
18+
import org.mockito.kotlin.doReturn
19+
import org.mockito.kotlin.eq
20+
import org.mockito.kotlin.mock
21+
import org.mockito.kotlin.never
22+
import org.mockito.kotlin.spy
23+
import org.mockito.kotlin.verify
24+
import org.mockito.kotlin.whenever
25+
import software.aws.toolkits.jetbrains.services.amazonq.AmazonQTestBase
26+
import software.aws.toolkits.jetbrains.services.codewhisperer.codescan.CodeWhispererCodeScanIssue
27+
import software.aws.toolkits.jetbrains.services.codewhisperer.codescan.CodeWhispererCodeScanManager
28+
import software.aws.toolkits.jetbrains.services.codewhisperer.codescan.Description
29+
import software.aws.toolkits.jetbrains.services.codewhisperer.codescan.Recommendation
30+
import software.aws.toolkits.jetbrains.services.codewhisperer.codescan.SuggestedFix
31+
import software.aws.toolkits.jetbrains.services.codewhisperer.util.CodeWhispererConstants
32+
33+
class BrowserConnectorTest : AmazonQTestBase() {
34+
private lateinit var browserConnector: BrowserConnector
35+
private lateinit var mockCodeScanManager: CodeWhispererCodeScanManager
36+
private lateinit var mockLocalFileSystem: LocalFileSystem
37+
private lateinit var mockFileDocumentManager: FileDocumentManager
38+
private lateinit var fixture: CodeInsightTestFixture
39+
40+
@Before
41+
override fun setup() {
42+
super.setup()
43+
fixture = projectRule.fixture
44+
45+
mockCodeScanManager = mock()
46+
mockLocalFileSystem = mock()
47+
mockFileDocumentManager = mock()
48+
49+
project.replaceService(CodeWhispererCodeScanManager::class.java, mockCodeScanManager, disposableRule.disposable)
50+
51+
browserConnector = spy(BrowserConnector(project = project))
52+
}
53+
54+
@Test
55+
fun `parseFindingsMessages should handle empty additionalMessages`() {
56+
val messagesMap = mapOf<String, Any>()
57+
58+
browserConnector.parseFindingsMessages(messagesMap)
59+
60+
verify(mockCodeScanManager, never()).addOnDemandIssues(any(), any(), any())
61+
}
62+
63+
@Test
64+
fun `parseFindingsMessages should handle null additionalMessages`() {
65+
val messagesMap = mapOf("additionalMessages" to null)
66+
67+
browserConnector.parseFindingsMessages(messagesMap)
68+
69+
verify(mockCodeScanManager, never()).addOnDemandIssues(any(), any(), any())
70+
}
71+
72+
@Test
73+
fun `parseFindingsMessages should filter messages with CODE_REVIEW_FINDINGS_SUFFIX`() {
74+
val findingsMessage = mapOf(
75+
"messageId" to "test_codeReviewFindings",
76+
"body" to """[{"filePath": "/test/file.kt", "issues": []}]"""
77+
)
78+
val otherMessage = mapOf(
79+
"messageId" to "other_message",
80+
"body" to "other content"
81+
)
82+
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage, otherMessage)
83+
val messagesMap = mapOf("additionalMessages" to additionalMessages)
84+
85+
browserConnector.parseFindingsMessages(messagesMap)
86+
87+
assert(additionalMessages.size == 1)
88+
assert(additionalMessages[0] == otherMessage)
89+
}
90+
91+
@Test
92+
fun `parseFindingsMessages should filter messages with DISPLAY_FINDINGS_SUFFIX`() {
93+
val findingsMessage = mapOf(
94+
"messageId" to "test_displayFindings",
95+
"body" to """[{"filePath": "/test/file.kt", "issues": []}]"""
96+
)
97+
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
98+
val messagesMap = mapOf("additionalMessages" to additionalMessages)
99+
100+
browserConnector.parseFindingsMessages(messagesMap)
101+
102+
assert(additionalMessages.isEmpty())
103+
}
104+
105+
@Test
106+
fun `parseFindingsMessages should process valid findings and verify mappedFindings populated`() {
107+
val mockVirtualFile = mock<VirtualFile> {
108+
on { isDirectory } doReturn false
109+
}
110+
val mockDocument = mock<Document> {
111+
on { lineCount } doReturn 5
112+
on { getLineStartOffset(0) } doReturn 0
113+
on { getLineEndOffset(0) } doReturn 10
114+
}
115+
116+
mockStatic(LocalFileSystem::class.java).use { localFileSystemMock ->
117+
localFileSystemMock.`when`<LocalFileSystem> { LocalFileSystem.getInstance() }.thenReturn(mockLocalFileSystem)
118+
whenever(mockLocalFileSystem.findFileByIoFile(any())) doReturn mockVirtualFile
119+
120+
mockStatic(FileDocumentManager::class.java).use { fileDocumentManagerMock ->
121+
fileDocumentManagerMock.`when`<FileDocumentManager> { FileDocumentManager.getInstance() } doReturn mockFileDocumentManager
122+
whenever(mockFileDocumentManager.getDocument(mockVirtualFile)) doReturn mockDocument
123+
whenever(mockCodeScanManager.isIgnoredIssue(any(), any(), any(), any())) doReturn false
124+
125+
val issue = BrowserConnector.FlareCodeScanIssue(
126+
startLine = 1, endLine = 1, comment = "Test comment", title = "Test Issue",
127+
description = Description("Test description", "Test text"), detectorId = "test-detector",
128+
detectorName = "Test Detector", findingId = "test-finding-id", ruleId = "test-rule",
129+
relatedVulnerabilities = listOf("CVE-2023-1234"), severity = "HIGH",
130+
recommendation = Recommendation("Fix this", "https://example.com"),
131+
suggestedFixes = listOf(SuggestedFix("Fix code", "Fixed code")),
132+
scanJobId = "test-job-id", language = "kotlin", autoDetected = false,
133+
filePath = "/test/file.kt", findingContext = "test context"
134+
)
135+
136+
val aggregatedIssue = BrowserConnector.AggregatedCodeScanIssue("/test/file.kt", listOf(issue))
137+
val findingsMessage = mapOf(
138+
"messageId" to "test_codeReviewFindings",
139+
"body" to Gson().toJson(listOf(aggregatedIssue))
140+
)
141+
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
142+
val messagesMap = mapOf("additionalMessages" to additionalMessages)
143+
144+
browserConnector.parseFindingsMessages(messagesMap)
145+
146+
val issuesCaptor = argumentCaptor<List<CodeWhispererCodeScanIssue>>()
147+
verify(mockCodeScanManager).addOnDemandIssues(
148+
issuesCaptor.capture(),
149+
any(),
150+
eq(CodeWhispererConstants.CodeAnalysisScope.AGENTIC)
151+
)
152+
153+
assert(additionalMessages.isEmpty())
154+
assert(issuesCaptor.firstValue.isNotEmpty())
155+
assert(issuesCaptor.firstValue[0].title == "Test Issue")
156+
}
157+
}
158+
}
159+
160+
@Test
161+
fun `parseFindingsMessages should skip directory files and not populate mappedFindings`() {
162+
val mockDirectoryFile = mock<VirtualFile> { on { isDirectory } doReturn true }
163+
164+
mockStatic(LocalFileSystem::class.java).use { localFileSystemMock ->
165+
localFileSystemMock.`when`<LocalFileSystem> { LocalFileSystem.getInstance() } doReturn mockLocalFileSystem
166+
whenever(mockLocalFileSystem.findFileByIoFile(any())) doReturn mockDirectoryFile
167+
168+
val issue = BrowserConnector.FlareCodeScanIssue(
169+
startLine = 1, endLine = 1, comment = null, title = "Test Issue",
170+
description = Description("Test description", "Test text"), detectorId = "test-detector",
171+
detectorName = "Test Detector", findingId = "test-finding-id", ruleId = null,
172+
relatedVulnerabilities = emptyList(), severity = "MEDIUM",
173+
recommendation = Recommendation("Fix this", "https://example.com"), suggestedFixes = emptyList(),
174+
scanJobId = "test-job-id", language = "kotlin", autoDetected = true,
175+
filePath = "/test/directory", findingContext = "test context"
176+
)
177+
178+
val aggregatedIssue = BrowserConnector.AggregatedCodeScanIssue("/test/directory", listOf(issue))
179+
val findingsMessage = mapOf(
180+
"messageId" to "test_displayFindings",
181+
"body" to Gson().toJson(listOf(aggregatedIssue))
182+
)
183+
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
184+
val messagesMap = mapOf("additionalMessages" to additionalMessages)
185+
186+
browserConnector.parseFindingsMessages(messagesMap)
187+
188+
val issuesCaptor = argumentCaptor<List<CodeWhispererCodeScanIssue>>()
189+
verify(mockCodeScanManager).addOnDemandIssues(
190+
issuesCaptor.capture(),
191+
any(),
192+
eq(CodeWhispererConstants.CodeAnalysisScope.AGENTIC)
193+
)
194+
195+
assert(issuesCaptor.firstValue.isEmpty())
196+
assert(additionalMessages.isEmpty())
197+
}
198+
}
199+
200+
@Test
201+
fun `parseFindingsMessages should handle invalid JSON gracefully`() {
202+
val findingsMessage = mapOf(
203+
"messageId" to "test_codeReviewFindings",
204+
"body" to "invalid json"
205+
)
206+
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
207+
val messagesMap = mapOf("additionalMessages" to additionalMessages)
208+
209+
browserConnector.parseFindingsMessages(messagesMap)
210+
211+
assert(additionalMessages.isEmpty())
212+
}
213+
}

0 commit comments

Comments
 (0)