Skip to content

Commit d78ec32

Browse files
authored
fix(amazonq): Fix cases where content may be incorrectly excluded from workspace. (#5482)
**Problem 1:** Incorrectly filtering out directories in `additionalGlobalIgnoreRulesForStrictSources` due to missing null check on file extension. Fixed and added tests. **Problem 2:** Because we start traversal at the content roots, but filter out any content not within the selection root, we were incorrectly stopping traversal when the selection root is nested within any content root. In this case, we were exiting without traversing to find the `selectedRoot`. Fixed and added tests. **Problem 3:** Add support for DevFile (as normally supported, when at workspace root) when workspace root is not a content root. Fixes excluding the DevFile when it's not in a content root.
1 parent b80479f commit d78ec32

File tree

6 files changed

+243
-125
lines changed

6 files changed

+243
-125
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type" : "bugfix",
3+
"description" : "Amazon Q: Fix cases where content may be incorrectly excluded from workspace."
4+
}

plugins/amazonq/chat/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/workspace/context/WorkspaceTest.kt

+100
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,19 @@
33

44
package software.aws.toolkits.jetbrains.services.amazonq.workspace.context
55

6+
import com.intellij.openapi.vcs.changes.ChangeListManager
67
import com.intellij.openapi.vfs.VirtualFile
78
import com.intellij.openapi.vfs.refreshAndFindVirtualDirectory
89
import com.intellij.openapi.vfs.refreshAndFindVirtualFile
910
import com.intellij.testFramework.LightPlatformTestCase
11+
import org.mockito.Mockito.mock
12+
import org.mockito.kotlin.doReturn
13+
import org.mockito.kotlin.whenever
14+
import software.aws.toolkits.jetbrains.services.amazonq.project.additionalGlobalIgnoreRules
15+
import software.aws.toolkits.jetbrains.services.amazonq.project.additionalGlobalIgnoreRulesForStrictSources
1016
import software.aws.toolkits.jetbrains.services.amazonq.project.findWorkspaceRoot
1117
import software.aws.toolkits.jetbrains.services.amazonq.project.isContentInWorkspace
18+
import software.aws.toolkits.jetbrains.services.amazonq.project.isWorkspaceSourceContent
1219
import java.nio.file.Files
1320
import java.nio.file.Path
1421
import kotlin.io.path.createDirectories
@@ -106,4 +113,97 @@ class WorkspaceTest : LightPlatformTestCase() {
106113

107114
assertFalse(isContentInWorkspace(testPath, setOf(projectPath, modulePath)))
108115
}
116+
117+
fun `test isWorkspaceSourceContent returns true for non-ignored file or directory with default ignore rules`() {
118+
val projectPath = createDir("test/project")
119+
val directoryPath = createDir("test/project/module")
120+
val codeFilePath = createDir("test/project/module/example.java")
121+
val nonCodeFilePath = createDir("test/project/module/example.bin")
122+
123+
val changeListManager = mock<ChangeListManager>().apply {
124+
whenever(isIgnoredFile(directoryPath)) doReturn false
125+
}
126+
127+
assertTrue(
128+
isWorkspaceSourceContent(
129+
directoryPath,
130+
setOf(projectPath),
131+
changeListManager,
132+
additionalGlobalIgnoreRules
133+
)
134+
)
135+
136+
assertTrue(
137+
isWorkspaceSourceContent(
138+
codeFilePath,
139+
setOf(projectPath),
140+
changeListManager,
141+
additionalGlobalIgnoreRules
142+
)
143+
)
144+
145+
assertTrue(
146+
isWorkspaceSourceContent(
147+
nonCodeFilePath,
148+
setOf(projectPath),
149+
changeListManager,
150+
additionalGlobalIgnoreRules
151+
)
152+
)
153+
154+
assertTrue(
155+
isWorkspaceSourceContent(
156+
directoryPath,
157+
setOf(projectPath),
158+
changeListManager,
159+
additionalGlobalIgnoreRulesForStrictSources
160+
)
161+
)
162+
163+
assertTrue(
164+
isWorkspaceSourceContent(
165+
codeFilePath,
166+
setOf(projectPath),
167+
changeListManager,
168+
additionalGlobalIgnoreRulesForStrictSources
169+
)
170+
)
171+
}
172+
173+
fun `test isWorkspaceSourceContent returns false for ignored file or directory with default ignore rules`() {
174+
val projectPath = createDir("test/project")
175+
val directoryPath = createDir("test/project/node_modules")
176+
val nonCodeFilePath = createDir("test/project/module/example.bin")
177+
178+
val changeListManager = mock<ChangeListManager>().apply {
179+
whenever(isIgnoredFile(directoryPath)) doReturn false
180+
}
181+
182+
assertFalse(
183+
isWorkspaceSourceContent(
184+
directoryPath,
185+
setOf(projectPath),
186+
changeListManager,
187+
additionalGlobalIgnoreRules
188+
)
189+
)
190+
191+
assertFalse(
192+
isWorkspaceSourceContent(
193+
directoryPath,
194+
setOf(projectPath),
195+
changeListManager,
196+
additionalGlobalIgnoreRulesForStrictSources
197+
)
198+
)
199+
200+
assertFalse(
201+
isWorkspaceSourceContent(
202+
nonCodeFilePath,
203+
setOf(projectPath),
204+
changeListManager,
205+
additionalGlobalIgnoreRulesForStrictSources
206+
)
207+
)
208+
}
109209
}

plugins/amazonq/chat/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonqFeatureDev/FeatureDevSessionContextTest.kt

+89-89
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,9 @@ import software.aws.toolkits.jetbrains.utils.rules.HeavyJavaCodeInsightTestFixtu
1515
import software.aws.toolkits.jetbrains.utils.rules.addFileToModule
1616
import java.util.zip.ZipFile
1717

18-
class FeatureDevSessionContextTest : FeatureDevTestBase(HeavyJavaCodeInsightTestFixtureRule()) {
19-
20-
private fun addFilesToProjectModule(vararg path: String) {
21-
val module = projectRule.module
22-
path.forEach { projectRule.fixture.addFileToModule(module, it, it) }
23-
}
18+
data class FileCase(val path: String, val content: String = "", val shouldInclude: Boolean = true)
2419

20+
class FeatureDevSessionContextTest : FeatureDevTestBase(HeavyJavaCodeInsightTestFixtureRule()) {
2521
@Rule
2622
@JvmField
2723
val ruleChain = RuleChain(projectRule, disposableRule)
@@ -35,90 +31,40 @@ class FeatureDevSessionContextTest : FeatureDevTestBase(HeavyJavaCodeInsightTest
3531
featureDevSessionContext = FeatureDevSessionContext(featureDevService.project, 1024)
3632
}
3733

38-
// FIXME: Add deeper tests, replacing previous shallow tests - BLOCKING
39-
40-
@Test
41-
fun testZipProjectWithoutAutoDev() {
42-
checkZipProject(
43-
false,
44-
setOf(
45-
"src/MyClass.java",
46-
"icons/menu.svg",
47-
"assets/header.jpg",
48-
"archive.zip",
49-
"output.bin",
50-
"gradle/wrapper/gradle-wrapper.jar",
51-
"gradle/wrapper/gradle-wrapper.properties",
52-
"images/logo.png",
53-
"builder/GetTestBuilder.java",
54-
"gradlew",
55-
"README.md",
56-
".gitignore",
57-
"License.md",
58-
"gradlew.bat",
59-
"license.txt",
60-
"build.gradle",
61-
"settings.gradle"
62-
)
63-
)
64-
}
65-
66-
@Test
67-
fun testZipProjectWithAutoDev() {
68-
checkZipProject(
69-
true,
70-
setOf(
71-
"src/MyClass.java",
72-
"icons/menu.svg",
73-
"assets/header.jpg",
74-
"archive.zip",
75-
"output.bin",
76-
"gradle/wrapper/gradle-wrapper.jar",
77-
"gradle/wrapper/gradle-wrapper.properties",
78-
"images/logo.png",
79-
"builder/GetTestBuilder.java",
80-
"gradlew",
81-
"README.md",
82-
".gitignore",
83-
"License.md",
84-
"gradlew.bat",
85-
"license.txt",
86-
"build.gradle",
87-
"devfile.yaml",
88-
"settings.gradle"
89-
)
90-
)
91-
}
34+
private fun fileCases(autoBuildEnabled: Boolean) = listOf(
35+
FileCase(path = ".gitignore"),
36+
FileCase(path = ".gradle/cached.jar", shouldInclude = false),
37+
FileCase(path = "src/MyClass.java"),
38+
FileCase(path = "gradlew"),
39+
FileCase(path = "gradlew.bat"),
40+
FileCase(path = "README.md"),
41+
FileCase(path = "settings.gradle"),
42+
FileCase(path = "build.gradle"),
43+
FileCase(path = "gradle/wrapper/gradle-wrapper.properties"),
44+
FileCase(path = "builder/GetTestBuilder.java"),
45+
FileCase(path = ".aws-sam/build/function1", shouldInclude = false),
46+
FileCase(path = ".gem/specs.rb", shouldInclude = false),
47+
FileCase(path = "archive.zip"),
48+
FileCase(path = "output.bin"),
49+
FileCase(path = "images/logo.png"),
50+
FileCase(path = "assets/header.jpg"),
51+
FileCase(path = "icons/menu.svg"),
52+
FileCase(path = "license.txt"),
53+
FileCase(path = "License.md"),
54+
FileCase(path = "node_modules/express", shouldInclude = false),
55+
FileCase(path = "build/outputs", shouldInclude = false),
56+
FileCase(path = "dist/bundle.js", shouldInclude = false),
57+
FileCase(path = "gradle/wrapper/gradle-wrapper.jar"),
58+
FileCase(path = "big-file.txt", content = "blob".repeat(1024 * 1024), shouldInclude = false),
59+
FileCase(path = "devfile.yaml", shouldInclude = autoBuildEnabled),
60+
)
9261

93-
fun checkZipProject(autoBuildEnabled: Boolean, expectedFiles: Set<String>) {
94-
addFilesToProjectModule(
95-
".gitignore",
96-
".gradle/cached.jar",
97-
"src/MyClass.java",
98-
"gradlew",
99-
"gradlew.bat",
100-
"README.md",
101-
"settings.gradle",
102-
"build.gradle",
103-
"gradle/wrapper/gradle-wrapper.properties",
104-
"builder/GetTestBuilder.java", // check for false positives
105-
".aws-sam/build/function1",
106-
".gem/specs.rb",
107-
"archive.zip",
108-
"output.bin",
109-
"images/logo.png",
110-
"assets/header.jpg",
111-
"icons/menu.svg",
112-
"license.txt",
113-
"License.md",
114-
"node_modules/express",
115-
"build/outputs",
116-
"dist/bundle.js",
117-
"gradle/wrapper/gradle-wrapper.jar",
118-
"devfile.yaml",
119-
)
62+
private fun checkZipProject(autoBuildEnabled: Boolean, fileCases: Iterable<FileCase>, onBeforeZip: (() -> Unit)? = null) {
63+
fileCases.forEach {
64+
projectRule.fixture.addFileToModule(module, it.path, it.content)
65+
}
12066

121-
projectRule.fixture.addFileToModule(module, "large-file.txt", "loblob".repeat(1024 * 1024))
67+
onBeforeZip?.invoke()
12268

12369
val zipResult = featureDevSessionContext.getProjectZip(autoBuildEnabled)
12470
val zipPath = zipResult.payload.path
@@ -132,6 +78,60 @@ class FeatureDevSessionContextTest : FeatureDevTestBase(HeavyJavaCodeInsightTest
13278
}
13379
}
13480

135-
assertEquals(zippedFiles, expectedFiles)
81+
// The input file paths are relative to the workspaceRoot, however the zip content is relative to the addressableRoot:
82+
val addressableRoot = featureDevSessionContext.addressableRoot.path
83+
val workspaceRoot = featureDevSessionContext.workspaceRoot.path
84+
val base = addressableRoot.removePrefix(workspaceRoot).removePrefix("/")
85+
fun addressablePathOf(path: String) = path.removePrefix(base).removePrefix("/")
86+
87+
fileCases.forEach {
88+
assertEquals(it.shouldInclude, zippedFiles.contains(addressablePathOf(it.path)))
89+
}
90+
}
91+
92+
@Test
93+
fun `test zip with autoBuild enabled`() {
94+
checkZipProject(autoBuildEnabled = true, fileCases(autoBuildEnabled = true))
95+
}
96+
97+
@Test
98+
fun `test zip with autoBuild disabled`() {
99+
checkZipProject(autoBuildEnabled = false, fileCases(autoBuildEnabled = false))
100+
}
101+
102+
@Test
103+
fun `test content is included when selection root is workspace root`() {
104+
val fileCases = listOf(
105+
FileCase(path = "file.txt", shouldInclude = true),
106+
FileCase(path = "project/file.txt", shouldInclude = true),
107+
FileCase(path = "deep/nested/file.txt", shouldInclude = true)
108+
)
109+
110+
checkZipProject(autoBuildEnabled = false, fileCases = fileCases, onBeforeZip = {
111+
featureDevSessionContext.selectionRoot = featureDevSessionContext.workspaceRoot
112+
})
113+
}
114+
115+
@Test
116+
fun `test content is included within selection root which is deeper than content root`() {
117+
val fileCases = listOf(FileCase(path = "project/module/deep/file.txt", shouldInclude = true))
118+
119+
checkZipProject(autoBuildEnabled = false, fileCases = fileCases, onBeforeZip = {
120+
featureDevSessionContext.selectionRoot = featureDevSessionContext.workspaceRoot.findFileByRelativePath("project/module/deep")
121+
?: error("Failed to find fixture")
122+
})
123+
}
124+
125+
@Test
126+
fun `test content is excluded outside of selection root`() {
127+
val fileCases = listOf(
128+
FileCase(path = "project/module/file.txt", shouldInclude = true),
129+
FileCase(path = "project/outside/no.txt", shouldInclude = false),
130+
)
131+
132+
checkZipProject(autoBuildEnabled = false, fileCases = fileCases, onBeforeZip = {
133+
featureDevSessionContext.selectionRoot = featureDevSessionContext.workspaceRoot.findFileByRelativePath("project/module")
134+
?: error("Failed to find fixture")
135+
})
136136
}
137137
}

0 commit comments

Comments
 (0)