-
Notifications
You must be signed in to change notification settings - Fork 275
fix(amazonq): capture more file events for workspace/ messages #5474
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
Merged
samgst-amazon
merged 6 commits into
feature/q-lsp
from
samgst/q-lsp-capture-more-events
Mar 13, 2025
Merged
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
9ae4880
capture more events for didCreate
samgst-amazon cca5b1e
Emit correct data for move and copy events
samgst-amazon b933941
Merge branch 'feature/q-lsp' into samgst/q-lsp-capture-more-events
samgst-amazon 5d326bc
detekt
samgst-amazon e3e0b85
orEmpty fix
samgst-amazon c30d884
Merge branch 'feature/q-lsp' into samgst/q-lsp-capture-more-events
samgst-amazon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,9 +9,11 @@ import com.intellij.openapi.application.ApplicationManager | |
| import com.intellij.openapi.components.serviceIfCreated | ||
| import com.intellij.openapi.project.Project | ||
| import com.intellij.openapi.vfs.VirtualFile | ||
| import com.intellij.openapi.vfs.newvfs.events.VFileCopyEvent | ||
| import com.intellij.openapi.vfs.newvfs.events.VFileCreateEvent | ||
| import com.intellij.openapi.vfs.newvfs.events.VFileDeleteEvent | ||
| import com.intellij.openapi.vfs.newvfs.events.VFileEvent | ||
| import com.intellij.openapi.vfs.newvfs.events.VFileMoveEvent | ||
| import com.intellij.openapi.vfs.newvfs.events.VFilePropertyChangeEvent | ||
| import com.intellij.util.messages.MessageBus | ||
| import com.intellij.util.messages.MessageBusConnection | ||
|
|
@@ -167,6 +169,32 @@ class WorkspaceServiceHandlerTest { | |
| verify(exactly = 0) { mockWorkspaceService.didCreateFiles(any()) } | ||
| } | ||
|
|
||
| @Test | ||
| fun `test didCreateFiles with move event`() = runTest { | ||
| val oldUri = URI("file:///test/oldPath") | ||
| val newUri = URI("file:///test/newPath") | ||
| val moveEvent = createMockVFileMoveEvent(oldUri, newUri, "test.py") | ||
|
|
||
| sut.after(listOf(moveEvent)) | ||
|
|
||
| val paramsSlot = slot<CreateFilesParams>() | ||
| verify { mockWorkspaceService.didCreateFiles(capture(paramsSlot)) } | ||
| assertEquals(normalizeFileUri(newUri.toString()), paramsSlot.captured.files[0].uri) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test didCreateFiles with copy event`() = runTest { | ||
| val originalUri = URI("file:///test/original") | ||
| val newUri = URI("file:///test/new") | ||
| val copyEvent = createMockVFileCopyEvent(originalUri, newUri, "test.py") | ||
|
|
||
| sut.after(listOf(copyEvent)) | ||
|
|
||
| val paramsSlot = slot<CreateFilesParams>() | ||
| verify { mockWorkspaceService.didCreateFiles(capture(paramsSlot)) } | ||
| assertEquals(normalizeFileUri(newUri.toString()), paramsSlot.captured.files[0].uri) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test didDeleteFiles with Python file`() = runTest { | ||
| val pyUri = URI("file:///test/path") | ||
|
|
@@ -237,6 +265,49 @@ class WorkspaceServiceHandlerTest { | |
| assertEquals(normalizeFileUri(dirUri.toString()), paramsSlot.captured.files[0].uri) | ||
| } | ||
|
|
||
|
|
||
| @Test | ||
| fun `test didDeleteFiles handles both delete and move events in same batch`() = runTest { | ||
| val deleteUri = URI("file:///test/deleteFile") | ||
| val oldMoveUri = URI("file:///test/oldMoveFile") | ||
| val newMoveUri = URI("file:///test/newMoveFile") | ||
|
|
||
| val deleteEvent = createMockVFileEvent(deleteUri, FileChangeType.Deleted, false, "py") | ||
| val moveEvent = createMockVFileMoveEvent(oldMoveUri, newMoveUri, "test.py") | ||
|
|
||
| sut.after(listOf(deleteEvent, moveEvent)) | ||
|
|
||
| val deleteParamsSlot = slot<DeleteFilesParams>() | ||
| verify { mockWorkspaceService.didDeleteFiles(capture(deleteParamsSlot)) } | ||
| assertEquals(2, deleteParamsSlot.captured.files.size) | ||
| assertEquals(normalizeFileUri(deleteUri.toString()), deleteParamsSlot.captured.files[0].uri) | ||
| assertEquals(normalizeFileUri(oldMoveUri.toString()), deleteParamsSlot.captured.files[1].uri) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test didDeleteFiles with move event of unsupported file type`() = runTest { | ||
| val oldUri = URI("file:///test/oldPath") | ||
| val newUri = URI("file:///test/newPath") | ||
| val moveEvent = createMockVFileMoveEvent(oldUri, newUri, "test.txt") | ||
|
|
||
| sut.after(listOf(moveEvent)) | ||
|
|
||
| verify(exactly = 0) { mockWorkspaceService.didDeleteFiles(any()) } | ||
| } | ||
|
|
||
| @Test | ||
| fun `test didDeleteFiles with move event of directory`() = runTest { | ||
| val oldUri = URI("file:///test/oldDir") | ||
| val newUri = URI("file:///test/newDir") | ||
| val moveEvent = createMockVFileMoveEvent(oldUri, newUri, "", true) | ||
|
|
||
| sut.after(listOf(moveEvent)) | ||
|
|
||
| val deleteParamsSlot = slot<DeleteFilesParams>() | ||
| verify { mockWorkspaceService.didDeleteFiles(capture(deleteParamsSlot)) } | ||
| assertEquals(normalizeFileUri(oldUri.toString()), deleteParamsSlot.captured.files[0].uri) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test didChangeWatchedFiles with valid events`() = runTest { | ||
| // Arrange | ||
|
|
@@ -262,6 +333,38 @@ class WorkspaceServiceHandlerTest { | |
| assertEquals(FileChangeType.Changed, paramsSlot.captured.changes[2].type) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test didChangeWatchedFiles with move event reports both delete and create`() = runTest { | ||
| val oldUri = URI("file:///test/oldPath") | ||
| val newUri = URI("file:///test/newPath") | ||
| val moveEvent = createMockVFileMoveEvent(oldUri, newUri, "test.py") | ||
|
|
||
| sut.after(listOf(moveEvent)) | ||
|
|
||
| val paramsSlot = slot<DidChangeWatchedFilesParams>() | ||
| verify { mockWorkspaceService.didChangeWatchedFiles(capture(paramsSlot)) } | ||
|
|
||
| assertEquals(2, paramsSlot.captured.changes.size) | ||
| assertEquals(normalizeFileUri(oldUri.toString()), paramsSlot.captured.changes[0].uri) | ||
| assertEquals(FileChangeType.Deleted, paramsSlot.captured.changes[0].type) | ||
| assertEquals(normalizeFileUri(newUri.toString()), paramsSlot.captured.changes[1].uri) | ||
| assertEquals(FileChangeType.Created, paramsSlot.captured.changes[1].type) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test didChangeWatchedFiles with copy event`() = runTest { | ||
| val originalUri = URI("file:///test/original") | ||
| val newUri = URI("file:///test/new") | ||
| val copyEvent = createMockVFileCopyEvent(originalUri, newUri, "test.py") | ||
|
|
||
| sut.after(listOf(copyEvent)) | ||
|
|
||
| val paramsSlot = slot<DidChangeWatchedFilesParams>() | ||
| verify { mockWorkspaceService.didChangeWatchedFiles(capture(paramsSlot)) } | ||
| assertEquals(normalizeFileUri(newUri.toString()), paramsSlot.captured.changes[0].uri) | ||
| assertEquals(FileChangeType.Created, paramsSlot.captured.changes[0].type) | ||
| } | ||
|
|
||
| @Test | ||
| fun `test no invoked messages when events are empty`() = runTest { | ||
| // Act | ||
|
|
@@ -510,20 +613,23 @@ class WorkspaceServiceHandlerTest { | |
| assertEquals("folder2", paramsSlot.captured.event.removed[0].name) | ||
| } | ||
|
|
||
| private fun createMockVFileEvent(uri: URI, type: FileChangeType = FileChangeType.Changed, isDirectory: Boolean, extension: String = "py"): VFileEvent { | ||
| private fun createMockVirtualFile(uri: URI, fileName: String, isDirectory: Boolean = false): VirtualFile { | ||
| val nioPath = mockk<Path> { | ||
| every { toUri() } returns uri | ||
| } | ||
| val virtualFile = mockk<VirtualFile> { | ||
| return mockk<VirtualFile> { | ||
| every { [email protected] } returns isDirectory | ||
| every { toNioPath() } returns nioPath | ||
| every { url } returns uri.path | ||
| every { path } returns "${uri.path}.$extension" | ||
| every { path } returns "${uri.path}/$fileName" | ||
| every { fileSystem } returns mockk { | ||
| every { protocol } returns "file" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun createMockVFileEvent(uri: URI, type: FileChangeType = FileChangeType.Changed, isDirectory: Boolean = false, extension: String = "py"): VFileEvent { | ||
| val virtualFile = createMockVirtualFile(uri, "test.$extension", isDirectory) | ||
| return when (type) { | ||
| FileChangeType.Deleted -> mockk<VFileDeleteEvent>() | ||
| FileChangeType.Created -> mockk<VFileCreateEvent>() | ||
|
|
@@ -533,49 +639,50 @@ class WorkspaceServiceHandlerTest { | |
| } | ||
| } | ||
|
|
||
| // for didRename events | ||
| private fun createMockPropertyChangeEvent( | ||
| oldName: String, | ||
| newName: String, | ||
| isDirectory: Boolean = false, | ||
| ): VFilePropertyChangeEvent { | ||
| val parentPath = mockk<Path>() | ||
| val filePath = mockk<Path>() | ||
| val oldUri = URI("file:///test/$oldName") | ||
| val newUri = URI("file:///test/$newName") | ||
| val file = createMockVirtualFile(newUri, newName, isDirectory) | ||
| every { file.parent } returns createMockVirtualFile(oldUri, oldName, isDirectory) | ||
|
|
||
| val parent = mockk<VirtualFile> { | ||
| every { toNioPath() } returns parentPath | ||
| every { [email protected] } returns isDirectory | ||
| every { path } returns "/test/$oldName" | ||
| every { url } returns "file:///test/$oldName" | ||
| every { fileSystem } returns mockk { | ||
| every { protocol } returns "file" | ||
| } | ||
| return mockk<VFilePropertyChangeEvent>().apply { | ||
| every { propertyName } returns VirtualFile.PROP_NAME | ||
| every { [email protected] } returns file | ||
| every { oldValue } returns oldName | ||
| every { newValue } returns newName | ||
| } | ||
| } | ||
|
|
||
| val file = mockk<VirtualFile> { | ||
| every { toNioPath() } returns filePath | ||
| every { [email protected] } returns parent | ||
| every { [email protected] } returns isDirectory | ||
| every { path } returns "/test/$newName" | ||
| every { url } returns "file:///test/$newName" | ||
| private fun createMockVFileMoveEvent(oldUri: URI, newUri: URI, fileName: String, isDirectory: Boolean = false): VFileMoveEvent { | ||
| val oldFile = createMockVirtualFile(oldUri, fileName, isDirectory) | ||
| val newFile = createMockVirtualFile(newUri, fileName, isDirectory) | ||
| return mockk<VFileMoveEvent>().apply { | ||
| every { file } returns newFile | ||
| every { oldPath } returns oldUri.path | ||
| every { oldParent } returns oldFile | ||
| } | ||
| } | ||
|
|
||
| private fun createMockVFileCopyEvent(originalUri: URI, newUri: URI, fileName: String): VFileCopyEvent { | ||
| val newParent = mockk<VirtualFile> { | ||
| every { findChild(any()) } returns createMockVirtualFile(newUri, fileName) | ||
| every { fileSystem } returns mockk { | ||
| every { protocol } returns "file" | ||
| } | ||
| } | ||
|
|
||
| every { parentPath.resolve(oldName) } returns mockk { | ||
| every { toUri() } returns URI("file:///test/$oldName") | ||
| } | ||
| every { filePath.toUri() } returns URI("file:///test/$newName") | ||
|
|
||
| return mockk<VFilePropertyChangeEvent>().apply { | ||
| every { propertyName } returns VirtualFile.PROP_NAME | ||
| every { [email protected] } returns file | ||
| every { oldValue } returns oldName | ||
| every { newValue } returns newName | ||
| return mockk<VFileCopyEvent>().apply { | ||
| every { file } returns createMockVirtualFile(originalUri, fileName) | ||
| every { [email protected] } returns newParent | ||
| every { newChildName } returns fileName | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
|
||
| // for windows unit tests | ||
| private fun normalizeFileUri(uri: String): String { | ||
| if (!System.getProperty("os.name").lowercase().contains("windows")) { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.