-
Notifications
You must be signed in to change notification settings - Fork 274
fix(Amazon Q): fix for test generation target file missing from payload. #5261
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
Changes from 12 commits
526d8c9
e182ec6
d943f8b
346cee3
556f3f1
a394c85
9276a46
e7544f0
f19f44a
7365162
37dc441
4662cca
62f896d
89545c2
1a8c609
a48f031
7d2979c
86efb24
812c790
12fc617
fd92af6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type" : "bugfix", | ||
| "description" : "Amazon Q /test: Fix for test generation payload creation to not filter out target file." | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
|
|
||
| import com.intellij.openapi.vfs.VirtualFile | ||
| import com.intellij.testFramework.RuleChain | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Assert.assertFalse | ||
| import org.junit.Assert.assertTrue | ||
| import org.junit.Before | ||
|
|
@@ -75,6 +76,7 @@ class FeatureDevSessionContextTest : FeatureDevTestBase(HeavyJavaCodeInsightTest | |
| @Test | ||
| fun testZipProject() { | ||
| addFilesToProjectModule( | ||
| ".gitignore", | ||
| ".gradle/cached.jar", | ||
| "src/MyClass.java", | ||
| "gradlew", | ||
|
|
@@ -83,6 +85,19 @@ class FeatureDevSessionContextTest : FeatureDevTestBase(HeavyJavaCodeInsightTest | |
| "settings.gradle", | ||
| "build.gradle", | ||
| "gradle/wrapper/gradle-wrapper.properties", | ||
| "builder/GetTestBuilder.java", // check for false positives | ||
| ".aws-sam/build/function1", | ||
| ".gem/specs.rb", | ||
| "archive.zip", | ||
| "output.bin", | ||
| "images/logo.png", | ||
| "assets/header.jpg", | ||
| "icons/menu.svg", | ||
| "license.txt", | ||
| "License.md", | ||
| "node_modules/express", | ||
| "build/outputs", | ||
| "dist/bundle.js" | ||
| ) | ||
|
|
||
| val zipResult = featureDevSessionContext.getProjectZip() | ||
|
|
@@ -102,11 +117,73 @@ class FeatureDevSessionContextTest : FeatureDevTestBase(HeavyJavaCodeInsightTest | |
| "gradlew", | ||
| "gradlew.bat", | ||
| "README.md", | ||
| "settings.gradle", | ||
| "build.gradle", | ||
| "gradle/wrapper/gradle-wrapper.properties", | ||
| "builder/GetTestBuilder.java" | ||
| ) | ||
|
|
||
| assertTrue(zippedFiles == expectedFiles) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test basic pattern conversion`() { | ||
| val input = "*.txt" | ||
| val expected = "(?:^|.*/?)[^/]*[^/]\\.txt(?:/.*)?\$" | ||
|
||
| assertEquals(expected, featureDevSessionContext.convertGitIgnorePatternToRegex(input)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test pattern with special characters`() { | ||
| val input = "test[abc].txt" | ||
| val expected = "(?:^|.*/?)test\\[abc\\]\\.txt(?:/.*)?$" | ||
| assertEquals(expected, featureDevSessionContext.convertGitIgnorePatternToRegex(input)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test pattern with double asterisk`() { | ||
| val input = "**/build" | ||
| val expected = "(?:^|.*/?).[^/]*[^/][^/]/build(?:/.*)?\$" | ||
| assertEquals(expected, featureDevSessionContext.convertGitIgnorePatternToRegex(input)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test pattern starting with slash`() { | ||
| val input = "/root/file.txt" | ||
| val expected = "^root/file\\.txt(?:/.*)?$" | ||
| assertEquals(expected, featureDevSessionContext.convertGitIgnorePatternToRegex(input)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test pattern ending with slash`() { | ||
| val input = "build/" | ||
| val expected = "(?:^|.*/?)build/.*" | ||
|
||
| assertEquals(expected, featureDevSessionContext.convertGitIgnorePatternToRegex(input)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test pattern with question mark`() { | ||
| val input = "file?.txt" | ||
| val expected = "(?:^|.*/?)file[^/]\\.txt(?:/.*)?$" | ||
| assertEquals(expected, featureDevSessionContext.convertGitIgnorePatternToRegex(input)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test complex pattern with multiple special characters`() { | ||
| val input = "**/test-[0-9]*.{java,kt}" | ||
| val expected = "(?:^|.*/?).[^/]*[^/][^/]/test-\\[0-9\\][^/]*[^/]\\.\\{java\\,kt\\}(?:/.*)?\$" | ||
| assertEquals(expected, featureDevSessionContext.convertGitIgnorePatternToRegex(input)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test empty pattern`() { | ||
| val input = "" | ||
| val expected = "(?:^|.*/?)(?:/.*)?$" | ||
| assertEquals(expected, featureDevSessionContext.convertGitIgnorePatternToRegex(input)) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test pattern with all special regex characters`() { | ||
| val input = ".$+()[]{}^|" | ||
| val expected = "(?:^|.*/?)\\.\\\$\\+\\(\\)\\[\\]\\{\\}\\^\\|(?:/.*)?$" | ||
| assertEquals(expected, featureDevSessionContext.convertGitIgnorePatternToRegex(input)) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ import kotlinx.coroutines.flow.channelFlow | |
| import kotlinx.coroutines.launch | ||
| import kotlinx.coroutines.runBlocking | ||
| import kotlinx.coroutines.withContext | ||
| import kotlinx.coroutines.withTimeout | ||
| import org.apache.commons.codec.digest.DigestUtils | ||
| import org.apache.commons.io.FileUtils | ||
| import software.aws.toolkits.jetbrains.core.coroutines.getCoroutineBgContext | ||
|
|
@@ -97,7 +98,7 @@ class FeatureDevSessionContext(val project: Project, val maxProjectSizeBytes: Lo | |
| addAll(additionalGitIgnoreRules.map { convertGitIgnorePatternToRegex(it) }) | ||
| addAll(parseGitIgnore()) | ||
| }.mapNotNull { pattern -> | ||
| runCatching { Regex(pattern) }.getOrNull() | ||
| runCatching { pattern?.let { Regex(it) } }.getOrNull() | ||
| } | ||
| } catch (e: Exception) { | ||
| emptyList() | ||
|
|
@@ -136,7 +137,13 @@ class FeatureDevSessionContext(val project: Project, val maxProjectSizeBytes: Lo | |
| // entries against them by adding a trailing /. | ||
| // TODO: Add unit tests for gitignore matching | ||
| val relative = if (path.startsWith(projectRootPath.toString())) Paths.get(path).relativeTo(projectRootPath) else path | ||
| async { pattern.matches("$relative/") } | ||
| async { | ||
| try { | ||
| withTimeout(REGEX_TIMEOUT_MS) { pattern.matches("$relative/") } | ||
| } catch (e: Exception) { | ||
| false | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -240,7 +247,7 @@ class FeatureDevSessionContext(val project: Project, val maxProjectSizeBytes: Lo | |
| tempFilePath | ||
| } | ||
|
|
||
| private fun parseGitIgnore(): Set<String> { | ||
| private fun parseGitIgnore(): Set<String?> { | ||
| if (!gitIgnoreFile.exists()) { | ||
| return emptySet() | ||
| } | ||
|
|
@@ -252,16 +259,59 @@ class FeatureDevSessionContext(val project: Project, val maxProjectSizeBytes: Lo | |
| } | ||
|
|
||
| // gitignore patterns are not regex, method update needed. | ||
| private fun convertGitIgnorePatternToRegex(pattern: String): String = pattern | ||
| .replace(".", "\\.") | ||
| .replace("*", ".*") | ||
| .let { if (it.endsWith("/")) "$it.*" else "$it/.*" } // Add a trailing /* to all patterns. (we add a trailing / to all files when matching) | ||
| fun convertGitIgnorePatternToRegex(pattern: String): String? { | ||
| // Skip invalid patterns for length check | ||
| if (pattern.length > MAX_PATTERN_LENGTH) { | ||
| return null | ||
|
||
| } | ||
|
|
||
| // Escape special regex characters except * and ? | ||
|
||
| var result = pattern | ||
| .replace(".", "\\.") | ||
| .replace("+", "\\+") | ||
| .replace("(", "\\(") | ||
| .replace(")", "\\)") | ||
| .replace("[", "\\[") | ||
| .replace("]", "\\]") | ||
| .replace("{", "\\{") | ||
| .replace("}", "\\}") | ||
| .replace(",", "\\,") | ||
| .replace("^", "\\^") | ||
| .replace("$", "\\$") | ||
| .replace("|", "\\|") | ||
|
|
||
| // Convert gitignore glob patterns to regex | ||
| result = result | ||
| .replace("**", ".*?") // Match any directory depth | ||
| .replace("*", "[^/]*?") // Match any character except path separator | ||
| .replace("?", "[^/]") // Match single character except path separator | ||
|
|
||
| // Handle start of pattern | ||
| result = if (result.startsWith("/")) { | ||
| "^${result.substring(1)}" | ||
| } else { | ||
| "(?:^|.*/?)$result" | ||
| } | ||
|
|
||
| // Handle end of pattern | ||
|
||
| result = if (result.endsWith("/")) { | ||
| "$result.*" | ||
| } else { | ||
| "$result(?:/.*)?$" | ||
| } | ||
|
|
||
| return result | ||
| } | ||
| var selectedSourceFolder: VirtualFile | ||
| set(newRoot) { | ||
| _selectedSourceFolder = newRoot | ||
| } | ||
| get() = _selectedSourceFolder | ||
|
|
||
| companion object { | ||
| private const val MAX_PATTERN_LENGTH = 256 // Maximum allowed pattern length | ||
| private const val REGEX_TIMEOUT_MS = 100L // Timeout for regex operations in milliseconds | ||
| } | ||
| } | ||
|
|
||
| data class ZipCreationResult(val payload: File, val checksum: String, val contentLength: Long) | ||
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.
Just noticed that
settings.gradleandbuild.gradleare being dropped. These should be included as a part of the zip for /dev.Is this the result of changing the gitignore used for the test or a change in the actual behavior of the function?
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 amend this for /dev.