Skip to content

fix(amazonq): fix LspUtilsTest in 241 #5522

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

fix(amazonq): fix LspUtilsTest in 241 #5522

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f9abdf3
fix test in 241
samgst-amazon Mar 28, 2025
4e7567c
detekt
samgst-amazon Mar 28, 2025
33e4810
spacing
samgst-amazon Mar 28, 2025
5fce0eb
Merge branch 'feature/q-lsp' into samgst/q-lsp-fix-241-test
samgst-amazon Apr 1, 2025
cee0d53
Merge branch 'feature/q-lsp' into samgst/q-lsp-fix-241-test
samgst-amazon Apr 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ...-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@
.forEach { zipEntry ->
val destPath = Paths.get(destDir.toString(), zipEntry.toString())
destPath.createParentDirectories()
Files.copy(zipEntry, destPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES)
Files.copy(zipEntry, destPath, StandardCopyOption.REPLACE_EXISTING)
val permissions = Files.getPosixFilePermissions(zipEntry)
Files.setPosixFilePermissions(destPath, permissions)

Check warning on line 96 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtils.kt#L94-L96 

Added lines #L94 - L96 were not covered by tests
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions ...munity/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/LspUtilsTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import software.aws.toolkits.jetbrains.utils.satisfiesKt
import java.nio.file.FileSystems
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.StandardCopyOption
import java.nio.file.attribute.PosixFilePermissions
import java.util.zip.ZipOutputStream
import kotlin.io.path.isRegularFile
Expand Down Expand Up @@ -86,7 +85,10 @@ class LspUtilsTest {
paths
.filter { it.isRegularFile() }
.forEach { file ->
Files.copy(file, zipfs.getPath("/").resolve(source.relativize(file).toString()), StandardCopyOption.COPY_ATTRIBUTES)
val targetPath = zipfs.getPath("/").resolve(source.relativize(file).toString())
Files.copy(file, targetPath)
val sourcePerms = Files.getPosixFilePermissions(file)
Files.setPosixFilePermissions(targetPath, sourcePerms)
Copy link
Contributor Author

@samgst-amazon samgst-amazon Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linux 241 fixed but now this breaks the windows builds:
LSP Error [UNZIP_FAILED]: Failed to extract zip file: Attributes of type java.nio.file.attribute.PosixFileAttributes not supported

Copy link
Contributor

@rli rli Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

odd this test should not be running on windows. but also it seems like 241 tests spontaneously fixed themselves?

Copy link
Contributor Author

@samgst-amazon samgst-amazon Mar 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

241 works here when the permission copying is split up like this in this PR, but from what I see in the feature branch commits 241 is still broken.

}
}
}
Expand Down
Loading