fix(Amazon Q): fix for test generation target file missing from payload. #5261

ashishrp-aws · 2025-01-14T17:44:46Z

… and fix for /test query for non-supported languages.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Description

Target File was getting filtered out due to featureDevSession Regex pattern matching based filtering. For example, a folder called builder is getting filtered out and the files in it because regex pattern include build.

fix to include target file regardless of filters.
fix for regex patterns to match correctly. for example, for build to filter out
build
build/
build/something
path/to/build
path/to/build/
path/to/build/something

and not filter out /builder/

fix for query for unsupported languages and external files

Checklist

My code follows the code style of this project
I have added tests to cover my changes
A short description of the change has been added to the CHANGELOG if the change is customer-facing in the IDE.
I have added metrics for my changes (if required)

License

I confirm that my contribution is made under the terms of the Apache 2.0 license.

… and fix for /test query for non-supported languages.

ashishrp-aws · 2025-01-14T19:23:02Z

/retryBuilds

rli · 2025-01-14T19:44:18Z

...s-community/src/software/aws/toolkits/jetbrains/services/amazonq/FeatureDevSessionContext.kt

@@ -218,9 +206,39 @@ class FeatureDevSessionContext(val project: Project, val maxProjectSizeBytes: Lo

    // gitignore patterns are not regex, method update needed.
    private fun convertGitIgnorePatternToRegex(pattern: String): String = pattern
+        // Escape special regex characters except * and ?


Added tests

rli · 2025-01-15T20:40:54Z

...t/software/aws/toolkits/jetbrains/services/amazonqFeatureDev/FeatureDevSessionContextTest.kt

+    @Test
+    fun `test pattern ending with slash`() {
+        val input = "build/"
+        val expected = "(?:^|.*/?)build/.*"


these patterns have polynomial complexity. should there be a filter on input to prevent DoS of user's machine?

also can we investigate what it would look like to use the same glob logic that git uses to match files? seems like that might be simpler than trying to build these patterns

Added checks for polynomial complexity and makes sense for glob logic. will take a look into it. Wanted to drop regex matching entirely for /test. Only reason still being used is to restrict payload size which is leading to payload size errors.

ashishrp-aws · 2025-01-15T22:52:06Z

/retryBuilds

neilk-aws · 2025-01-15T23:52:35Z

...t/software/aws/toolkits/jetbrains/services/amazonqFeatureDev/FeatureDevSessionContextTest.kt

+    @Test
+    fun `test basic pattern conversion`() {
+        val input = "*.txt"
+        val expected = "(?:^|.*/?)[^/]*[^/]\\.txt(?:/.*)?\$"


How do I validate if this is correct?

This appears to be a snapshot test of what is built versus a functional test that demonstrates that what is an example file that would've been dropped is no longer dropped

we can test for pattern matching as well. That's why i included more files in the project zip test to validate it. Including false positives.

My point is that these tests do little to validate behaviors and protect against regressions. If we need to update the logic, how does one go about constructing the expected value?

Regardless, outside of the zip test above, I'd like to see more examples of each of these type of files and how they respond to the presence of such inputs in the gitignore.

neilk-aws · 2025-01-15T23:53:38Z

...s-community/src/software/aws/toolkits/jetbrains/services/amazonq/FeatureDevSessionContext.kt

+            "(?:^|.*/?)$result"
+        }
+
+        // Handle end of pattern


What is this doing? How do I know if the behavior is comprehensive?

to handle folder structures in patterns. for /src and src/ cases

Its too match all contents inside the directory in case of src/ and glob matching to check for .svg and .png and other patterns everywhere and not just in root.

neilk-aws · 2025-01-15T23:54:58Z

...s-community/src/software/aws/toolkits/jetbrains/services/amazonq/FeatureDevSessionContext.kt

+    fun convertGitIgnorePatternToRegex(pattern: String): String? {
+        // Skip invalid patterns for length check
+        if (pattern.length > MAX_PATTERN_LENGTH) {
+            return null


What happens when this returns null? Why was this value decided to be 256?

256 is generally placed limit. If it returns null , nothing happens pattern matching skips incase of null.

Is a different way of saying this that if the pattern is longer than 256 characters, then we will make no effort to exclude it and will always include it in the zip?

ashishrp-aws · 2025-01-16T01:23:01Z

/retryBuilds

neilk-aws · 2025-01-16T04:49:04Z

...t/software/aws/toolkits/jetbrains/services/amazonqFeatureDev/FeatureDevSessionContextTest.kt

@@ -102,11 +117,73 @@ class FeatureDevSessionContextTest : FeatureDevTestBase(HeavyJavaCodeInsightTest
            "gradlew",
            "gradlew.bat",
            "README.md",
-            "settings.gradle",


Just noticed that settings.gradle and build.gradle are 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?

Can amend this for /dev.

neilk-aws · 2025-01-16T04:53:12Z

...t/software/aws/toolkits/jetbrains/services/amazonqFeatureDev/FeatureDevSessionContextTest.kt

+    @Test
+    fun `test basic pattern conversion`() {
+        val input = "*.txt"
+        val expected = "(?:^|.*/?)[^/]*[^/]\\.txt(?:/.*)?\$"


My point is that these tests do little to validate behaviors and protect against regressions. If we need to update the logic, how does one go about constructing the expected value?

Regardless, outside of the zip test above, I'd like to see more examples of each of these type of files and how they respond to the presence of such inputs in the gitignore.

neilk-aws · 2025-01-16T04:56:01Z

...s-community/src/software/aws/toolkits/jetbrains/services/amazonq/FeatureDevSessionContext.kt

+    fun convertGitIgnorePatternToRegex(pattern: String): String? {
+        // Skip invalid patterns for length check
+        if (pattern.length > MAX_PATTERN_LENGTH) {
+            return null


Is a different way of saying this that if the pattern is longer than 256 characters, then we will make no effort to exclude it and will always include it in the zip?

ashishrp-aws · 2025-01-16T05:45:02Z

#5261 (comment)
I have included as many file names i could in the tests.

ashishrp-aws · 2025-01-16T05:55:22Z

#5261 (comment)
Yes. checks needed as per richard's comments

Fix for Test generation zip to include target file, Regex pattern fix…

526d8c9

… and fix for /test query for non-supported languages.

ashishrp-aws requested review from a team as code owners January 14, 2025 17:44

ashishrp-aws added 2 commits January 14, 2025 09:46

Merge branch 'main' into workspace-test

e182ec6

Merge branch 'main' into workspace-test

d943f8b

ashishrp-aws enabled auto-merge (squash) January 14, 2025 18:05

chungjac approved these changes Jan 14, 2025

View reviewed changes

Merge branch 'main' into workspace-test

346cee3

rli reviewed Jan 14, 2025

View reviewed changes

ashishrp-aws and others added 2 commits January 15, 2025 08:34

Merge branch 'main' into workspace-test

556f3f1

Removing FeatureDevSessionContext changes.

a394c85

ashishrp-aws requested a review from a team as a code owner January 15, 2025 20:15

ashishrp-aws and others added 2 commits January 15, 2025 12:15

Merge branch 'main' into workspace-test

9276a46

Added comment explaining file inclusion.

e7544f0

rli reviewed Jan 15, 2025

View reviewed changes

ashishrp-aws and others added 3 commits January 15, 2025 13:06

Merge branch 'main' into workspace-test

f19f44a

Added Timeout and length check for pattern matching

7365162

detekt correction.

37dc441

detekt Main correction for Nameshadowing.

4662cca

neilk-aws requested changes Jan 15, 2025

View reviewed changes

neilk-aws reviewed Jan 16, 2025

View reviewed changes

ashishrp-aws and others added 3 commits January 16, 2025 12:26

Merge branch 'main' into workspace-test

62f896d

Reverting the FeatureDevSessionContext pattern matching chnages.

89545c2

Correcting the chnages for FeatureDevSessionContext

1a8c609

neilk-aws approved these changes Jan 16, 2025

View reviewed changes

Merge branch 'main' into workspace-test

a48f031

rli approved these changes Jan 17, 2025

View reviewed changes

manodnyab approved these changes Jan 17, 2025

View reviewed changes

ashishrp-aws added 5 commits January 17, 2025 11:27

Merge branch 'main' into workspace-test

7d2979c

Merge branch 'main' into workspace-test

86efb24

Merge branch 'main' into workspace-test

812c790

Merge branch 'main' into workspace-test

12fc617

Merge branch 'main' into workspace-test

fd92af6

ashishrp-aws merged commit ecc73fe into aws:main Jan 21, 2025
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(Amazon Q): fix for test generation target file missing from payload. #5261

fix(Amazon Q): fix for test generation target file missing from payload. #5261

ashishrp-aws commented Jan 14, 2025 •

edited

Loading

ashishrp-aws commented Jan 14, 2025

rli Jan 14, 2025

ashishrp-aws Jan 14, 2025

ashishrp-aws Jan 15, 2025

rli Jan 15, 2025

rli Jan 15, 2025

ashishrp-aws Jan 15, 2025 •

edited

Loading

ashishrp-aws commented Jan 15, 2025

neilk-aws Jan 15, 2025

ashishrp-aws Jan 16, 2025 •

edited

Loading

neilk-aws Jan 16, 2025

neilk-aws Jan 15, 2025

ashishrp-aws Jan 16, 2025

ashishrp-aws Jan 16, 2025

neilk-aws Jan 15, 2025

ashishrp-aws Jan 16, 2025

neilk-aws Jan 16, 2025

ashishrp-aws commented Jan 16, 2025

neilk-aws Jan 16, 2025

ashishrp-aws Jan 16, 2025

neilk-aws Jan 16, 2025

neilk-aws Jan 16, 2025

ashishrp-aws commented Jan 16, 2025

ashishrp-aws commented Jan 16, 2025 •

edited

Loading

fix(Amazon Q): fix for test generation target file missing from payload. #5261

fix(Amazon Q): fix for test generation target file missing from payload. #5261

Conversation

ashishrp-aws commented Jan 14, 2025 • edited Loading

Types of changes

Description

Checklist

License

ashishrp-aws commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashishrp-aws Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

ashishrp-aws commented Jan 15, 2025

Choose a reason for hiding this comment

ashishrp-aws Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashishrp-aws commented Jan 16, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashishrp-aws commented Jan 16, 2025

ashishrp-aws commented Jan 16, 2025 • edited Loading

ashishrp-aws commented Jan 14, 2025 •

edited

Loading

ashishrp-aws Jan 15, 2025 •

edited

Loading

ashishrp-aws Jan 16, 2025 •

edited

Loading

ashishrp-aws commented Jan 16, 2025 •

edited

Loading