From 25f97b241830e24609945ae891342dadde6054d7 Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Thu, 30 Jul 2026 11:53:59 -0700 Subject: [PATCH 1/3] ADFA-4936: Rewrite plugin-manager unit tests against current service API The nightly Jacoco/SonarQube workflow failed at :plugin-manager:compileV8DebugUnitTestKotlin: the tests referenced a service model removed in an earlier refactor (FileServiceImpl/ProjectServiceImpl/ResourceServiceImpl and IdeResourceService). Because the sonarqube task depends on jacocoAggregateReport which depends on every subproject's testV8DebugUnitTest, this compile failure skipped the aggregate report (the "no files found for jacocoAggregateReport" upload warning) and failed the run. The impls were rewritten to a pluginId+permissions model (IdeFileServiceImpl, IdeProjectServiceImpl) that takes File args and returns raw types; the Resource service was removed outright. Rewrite the tests against the current API: - IdeFileServiceImplTest (replaces FileServiceImplTest): construct IdeFileServiceImpl with a permissive PathValidator (the production allowlist only permits fixed on-device roots) and FILESYSTEM_WRITE; cover write/read/replace/delete/list plus disallowed-path and missing-permission SecurityException. - E2EIntegrationTest: file-workflow-through-registry against the current API. - PluginManagerIntegrationTest: service registration/retrieval + functional file ops + path-security. - Delete ResourceServiceImplTest: the Resource service no longer exists, nothing to rewrite to. Verified: :plugin-manager:testV8DebugUnitTest -> 13 tests, 0 failures. --- .../plugins/manager/E2EIntegrationTest.kt | 117 ++++----- .../core/PluginManagerIntegrationTest.kt | 246 ++++++++---------- .../manager/services/FileServiceImplTest.kt | 111 -------- .../services/IdeFileServiceImplTest.kt | 116 +++++++++ .../services/ResourceServiceImplTest.kt | 42 --- 5 files changed, 268 insertions(+), 364 deletions(-) delete mode 100644 plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/FileServiceImplTest.kt create mode 100644 plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeFileServiceImplTest.kt delete mode 100644 plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/ResourceServiceImplTest.kt diff --git a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/E2EIntegrationTest.kt b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/E2EIntegrationTest.kt index 6f5e0cb272..9fbd8e7608 100644 --- a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/E2EIntegrationTest.kt +++ b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/E2EIntegrationTest.kt @@ -1,87 +1,64 @@ package com.itsaky.androidide.plugins.manager -import com.itsaky.androidide.plugins.services.IdeFileService -import com.itsaky.androidide.plugins.services.IdeResourceService +import com.itsaky.androidide.plugins.PluginPermission import com.itsaky.androidide.plugins.manager.context.ServiceRegistryImpl -import com.itsaky.androidide.plugins.manager.services.FileServiceImpl -import com.itsaky.androidide.plugins.manager.services.ProjectServiceImpl -import com.itsaky.androidide.plugins.manager.services.ResourceServiceImpl -import org.junit.Test -import org.junit.Assert.* -import org.junit.Before +import com.itsaky.androidide.plugins.manager.services.IdeFileServiceImpl +import com.itsaky.androidide.plugins.services.IdeFileService import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test import java.io.File class E2EIntegrationTest { + private lateinit var tempProjectRoot: File - private lateinit var tempProjectRoot: File - - @Before - fun setUp() { - tempProjectRoot = File.createTempFile("e2e-test-", "").apply { - delete() - mkdirs() - } - } - - @After - fun tearDown() { - tempProjectRoot.deleteRecursively() - } - - @Test - fun testCompletePluginWorkflow() { - // Simulate realistic plugin scenario: - // 1. Create a source file - // 2. Add a dependency to build.gradle.kts - // 3. Read resource strings - - val serviceRegistry = ServiceRegistryImpl() - val fileService = FileServiceImpl(tempProjectRoot) - val projectService = ProjectServiceImpl(tempProjectRoot) - val resourceService = ResourceServiceImpl(tempProjectRoot) + @Before + fun setUp() { + tempProjectRoot = + File.createTempFile("e2e-test-", "").apply { + delete() + mkdirs() + } + } - serviceRegistry.register(IdeFileService::class.java, fileService) - serviceRegistry.register(IdeProjectService::class.java, projectService) - serviceRegistry.register(IdeResourceService::class.java, resourceService) + @After + fun tearDown() { + tempProjectRoot.deleteRecursively() + } - // Step 1: Create source file via FileService - val createResult = fileService.createFile( - "app/src/main/kotlin/com/example/Main.kt", - "package com.example\n\nfun main() {\n println(\"Hello\")\n}" - ) - assertTrue("Failed to create source file: ${createResult.error}", createResult.success) + @Test + fun fileServiceWorkflowThroughRegistry() { + val registry = ServiceRegistryImpl() + val fileService = + IdeFileServiceImpl( + pluginId = "e2e-plugin", + permissions = setOf(PluginPermission.FILESYSTEM_WRITE), + pathValidator = + object : IdeFileServiceImpl.PathValidator { + override fun isPathAllowed(path: File) = path.absolutePath.startsWith(tempProjectRoot.absolutePath) - // Step 2: Add dependency via ProjectService - val buildFile = File(tempProjectRoot, "app/build.gradle.kts") - buildFile.parentFile?.mkdirs() - buildFile.writeText(""" - dependencies { - implementation("androidx.core:core-ktx:1.9.0") - } - """.trimIndent()) + override fun getAllowedPaths() = listOf(tempProjectRoot.absolutePath) + }, + ) + registry.register(IdeFileService::class.java, fileService) - val depResult = projectService.addDependency( - "app/build.gradle.kts", - "implementation(\"com.google.code.gson:gson:2.10.1\")" - ) - assertTrue("Failed to add dependency: ${depResult.error}", depResult.success) + val resolved = registry.get(IdeFileService::class.java) + assertNotNull("IdeFileService should be retrievable from the registry", resolved) - val updatedBuildContent = buildFile.readText() - assertTrue("Dependency not added to build file", updatedBuildContent.contains("gson:2.10.1")) + val sourceFile = File(tempProjectRoot, "app/src/main/kotlin/com/example/Main.kt") + val source = "package com.example\n\nfun main() {\n println(\"Hello\")\n}" - // Step 3: Access resource via ResourceService - val stringResult = resourceService.getString("app_name") - assertTrue("Failed to get string resource: ${stringResult.error}", stringResult.success) - assertNotNull("String resource data is null", stringResult.data) + assertTrue("write should succeed", resolved!!.writeFile(sourceFile, source)) + assertEquals(source, resolved.readFile(sourceFile)) - // Step 4: Verify file service can read created file - val readResult = fileService.readFile("app/src/main/kotlin/com/example/Main.kt") - assertTrue("Failed to read created file: ${readResult.error}", readResult.success) - assertTrue("File content mismatch", readResult.data!!.contains("package com.example")) + val listed = resolved.listFiles(sourceFile.parentFile, recursive = false).map { it.name } + assertTrue("listing should include the created file", listed.contains("Main.kt")) - // Step 5: Cleanup via FileService - val deleteResult = fileService.deleteFile("app/src/main/kotlin/com/example/Main.kt") - assertTrue("Failed to delete file: ${deleteResult.error}", deleteResult.success) - } + assertTrue("delete should succeed", resolved.delete(sourceFile)) + assertFalse("deleted file should be gone", sourceFile.exists()) + } } diff --git a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManagerIntegrationTest.kt b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManagerIntegrationTest.kt index 0be557c25c..61f6a3e52e 100644 --- a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManagerIntegrationTest.kt +++ b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManagerIntegrationTest.kt @@ -1,153 +1,117 @@ package com.itsaky.androidide.plugins.manager.core -import com.itsaky.androidide.plugins.PluginLogger +import com.itsaky.androidide.plugins.PluginPermission import com.itsaky.androidide.plugins.ServiceRegistry +import com.itsaky.androidide.plugins.extensions.IProject import com.itsaky.androidide.plugins.manager.context.ServiceRegistryImpl -import com.itsaky.androidide.plugins.manager.services.FileServiceImpl -import com.itsaky.androidide.plugins.manager.services.ProjectServiceImpl -import com.itsaky.androidide.plugins.manager.services.ResourceServiceImpl +import com.itsaky.androidide.plugins.manager.services.IdeFileServiceImpl +import com.itsaky.androidide.plugins.manager.services.IdeProjectServiceImpl import com.itsaky.androidide.plugins.services.IdeFileService -import com.itsaky.androidide.plugins.services.IdeResourceService -import org.junit.Test -import org.junit.Assert.* -import org.junit.Before +import com.itsaky.androidide.plugins.services.IdeProjectService import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test import java.io.File /** - * Integration tests for PluginManager service registration and retrieval. - * These tests verify that services are properly registered and can be retrieved - * through the service registry, mimicking how PluginManager registers services. + * Integration tests for plugin service registration and retrieval, mirroring how + * PluginManager registers services into the [ServiceRegistry] and how plugins then + * resolve and use them. */ class PluginManagerIntegrationTest { - - private lateinit var tempProjectRoot: File - private lateinit var serviceRegistry: ServiceRegistry - - @Before - fun setup() { - // Create a temporary project root directory - tempProjectRoot = File.createTempFile("test_project_", "").apply { - delete() - mkdirs() - } - - // Create a build.gradle file for project service tests - val buildFile = File(tempProjectRoot, "build.gradle") - buildFile.writeText(""" - plugins { - id 'java' - } - - dependencies { - } - """.trimIndent()) - - // Create a test file with content - val testFile = File(tempProjectRoot, "test.txt") - testFile.writeText("Integration test content") - - // Initialize service registry - serviceRegistry = ServiceRegistryImpl() - } - - @After - fun cleanup() { - // Clean up temporary directory - if (tempProjectRoot.exists()) { - tempProjectRoot.deleteRecursively() - } - } - - /** - * Test that services can be registered and retrieved through the service registry. - * This simulates how PluginManager registers services in createPluginContext() - * (lines 1292-1323 of PluginManager.kt). - */ - @Test - fun testServiceRegistration() { - // Simulate PluginManager's service registration for Phase 2 services - // This mirrors the code in PluginManager.createPluginContext() lines 1292-1323 - - // Register IdeFileService - val fileService = FileServiceImpl(tempProjectRoot) - serviceRegistry.register(IdeFileService::class.java, fileService) - - // Register IdeProjectService - val projectService = ProjectServiceImpl(tempProjectRoot) - serviceRegistry.register(IdeProjectService::class.java, projectService) - - // Register IdeResourceService - val resourceService = ResourceServiceImpl(tempProjectRoot) - serviceRegistry.register(IdeResourceService::class.java, resourceService) - - // Verify services can be retrieved through the registry - val retrievedFileService = serviceRegistry.get(IdeFileService::class.java) - assertNotNull("IdeFileService should be registered and retrievable", retrievedFileService) - assertSame("Retrieved service should be the same instance", fileService, retrievedFileService) - - val retrievedProjectService = serviceRegistry.get(IdeProjectService::class.java) - assertNotNull("IdeProjectService should be registered and retrievable", retrievedProjectService) - assertSame("Retrieved service should be the same instance", projectService, retrievedProjectService) - - val retrievedResourceService = serviceRegistry.get(IdeResourceService::class.java) - assertNotNull("IdeResourceService should be registered and retrievable", retrievedResourceService) - assertSame("Retrieved service should be the same instance", resourceService, retrievedResourceService) - } - - /** - * Test that FileService retrieved through the service registry is functional. - * This verifies the complete integration: registration -> retrieval -> usage. - */ - @Test - fun testFileServiceIntegration() { - // Register the service (simulating PluginManager) - val fileService = FileServiceImpl(tempProjectRoot) - serviceRegistry.register(IdeFileService::class.java, fileService) - - // Retrieve service through the registry (how plugins would access it) - val retrievedService = serviceRegistry.get(IdeFileService::class.java) - assertNotNull("Service should be retrievable from registry", retrievedService) - - // Test basic file operations through the retrieved service - - // 1. Read existing file - val readResult = retrievedService!!.readFile("test.txt") - assertTrue("File read should succeed", readResult.success) - assertEquals("Integration test content", readResult.data) - - // 2. Create new file - val createResult = retrievedService.createFile("new_file.txt", "New content") - assertTrue("File creation should succeed", createResult.success) - val newFile = File(tempProjectRoot, "new_file.txt") - assertTrue("New file should exist on disk", newFile.exists()) - assertEquals("New content", newFile.readText()) - - // 3. Update existing file - val updateResult = retrievedService.updateFile("test.txt", "Updated content") - assertTrue("File update should succeed", updateResult.success) - val testFile = File(tempProjectRoot, "test.txt") - assertEquals("Updated content", testFile.readText()) - - // 4. List files - val listResult = retrievedService.listFiles(".", false) - assertTrue("File listing should succeed", listResult.success) - assertNotNull("File list should not be null", listResult.data) - val fileList = listResult.data!! - assertTrue("File list should contain test.txt", fileList.contains("test.txt")) - assertTrue("File list should contain new_file.txt", fileList.contains("new_file.txt")) - - // 5. Delete file - val deleteResult = retrievedService.deleteFile("new_file.txt") - assertTrue("File deletion should succeed", deleteResult.success) - assertFalse("Deleted file should not exist", newFile.exists()) - - // 6. Verify security: path traversal prevention - val traversalResult = retrievedService.readFile("../outside.txt") - assertFalse("Path traversal should be prevented", traversalResult.success) - assertTrue( - "Error should mention path traversal", - traversalResult.error?.contains("Path traversal") == true - ) - } + private lateinit var tempProjectRoot: File + private lateinit var serviceRegistry: ServiceRegistry + + @Before + fun setup() { + tempProjectRoot = + File.createTempFile("test_project_", "").apply { + delete() + mkdirs() + } + File(tempProjectRoot, "test.txt").writeText("Integration test content") + serviceRegistry = ServiceRegistryImpl() + } + + @After + fun cleanup() { + tempProjectRoot.deleteRecursively() + } + + @Test + fun servicesCanBeRegisteredAndRetrieved() { + val fileService = fileService() + serviceRegistry.register(IdeFileService::class.java, fileService) + + val projectService = + IdeProjectServiceImpl( + pluginId = "test-plugin", + permissions = setOf(PluginPermission.FILESYSTEM_READ), + projectProvider = EmptyProjectProvider, + ) + serviceRegistry.register(IdeProjectService::class.java, projectService) + + val resolvedFile = serviceRegistry.get(IdeFileService::class.java) + assertNotNull("IdeFileService should be registered and retrievable", resolvedFile) + assertSame("Retrieved service should be the same instance", fileService, resolvedFile) + + val resolvedProject = serviceRegistry.get(IdeProjectService::class.java) + assertNotNull("IdeProjectService should be registered and retrievable", resolvedProject) + assertSame("Retrieved service should be the same instance", projectService, resolvedProject) + } + + @Test + fun fileServiceIsFunctionalThroughRegistry() { + serviceRegistry.register(IdeFileService::class.java, fileService()) + val service = serviceRegistry.get(IdeFileService::class.java) + assertNotNull("Service should be retrievable from registry", service) + + val existing = File(tempProjectRoot, "test.txt") + assertEquals("Integration test content", service!!.readFile(existing)) + + val created = File(tempProjectRoot, "new_file.txt") + assertTrue("write should succeed", service.writeFile(created, "New content")) + assertEquals("New content", created.readText()) + + assertTrue("replace should succeed", service.replaceInFile(existing, "Integration", "Updated")) + assertEquals("Updated test content", existing.readText()) + + val names = service.listFiles(tempProjectRoot, recursive = false).map { it.name } + assertTrue("listing should contain test.txt", names.contains("test.txt")) + assertTrue("listing should contain new_file.txt", names.contains("new_file.txt")) + + assertTrue("delete should succeed", service.delete(created)) + assertFalse("deleted file should be gone", created.exists()) + } + + @Test(expected = SecurityException::class) + fun fileServiceRejectsPathsOutsideProject() { + serviceRegistry.register(IdeFileService::class.java, fileService()) + serviceRegistry.get(IdeFileService::class.java)!!.readFile(File("/etc/passwd")) + } + + private fun fileService() = + IdeFileServiceImpl( + pluginId = "test-plugin", + permissions = setOf(PluginPermission.FILESYSTEM_WRITE), + pathValidator = + object : IdeFileServiceImpl.PathValidator { + override fun isPathAllowed(path: File) = path.absolutePath.startsWith(tempProjectRoot.absolutePath) + + override fun getAllowedPaths() = listOf(tempProjectRoot.absolutePath) + }, + ) + + private object EmptyProjectProvider : IdeProjectServiceImpl.ProjectProvider { + override fun getCurrentProject(): IProject? = null + + override fun getAllProjects(): List = emptyList() + + override fun getProjectByPath(path: File): IProject? = null + } } diff --git a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/FileServiceImplTest.kt b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/FileServiceImplTest.kt deleted file mode 100644 index 32d8f683f0..0000000000 --- a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/FileServiceImplTest.kt +++ /dev/null @@ -1,111 +0,0 @@ -package com.itsaky.androidide.plugins.manager.services - -import com.itsaky.androidide.plugins.services.IdeFileService -import org.junit.Test -import org.junit.Assert.* -import org.junit.Before -import java.io.File - -class FileServiceImplTest { - - private lateinit var projectRoot: File - - @Before - fun setUp() { - // Create a unique temporary directory for each test - projectRoot = File.createTempFile("test-project-", "").parentFile!! - projectRoot.deleteRecursively() - projectRoot.mkdirs() - } - - @Test - fun testReadFileRejectsPathTraversal() { - val service = FileServiceImpl(projectRoot) - val result = service.readFile("../../etc/passwd") - - assertFalse(result.success) - assertNotNull(result.error) - assertTrue(result.error!!.contains("Path traversal")) - } - - @Test - fun testReadFileSuccess() { - val testFile = File(projectRoot, "test.txt") - testFile.writeText("Hello World") - - val service = FileServiceImpl(projectRoot) - val result = service.readFile("test.txt") - - assertTrue(result.success) - assertEquals("Hello World", result.data) - assertNull(result.error) - } - - @Test - fun testCreateFileSuccess() { - val service = FileServiceImpl(projectRoot) - - val result = service.createFile("new.txt", "Content") - - assertTrue("Result was not successful: ${result.error}", result.success) - assertTrue("File does not exist at ${File(projectRoot, "new.txt").absolutePath}", File(projectRoot, "new.txt").exists()) - } - - @Test - fun testUpdateFileSuccess() { - val testFile = File(projectRoot, "test.txt") - testFile.writeText("Original") - - val service = FileServiceImpl(projectRoot) - val result = service.updateFile("test.txt", "Updated") - - assertTrue(result.success) - assertEquals("Updated", testFile.readText()) - } - - @Test - fun testDeleteFileSuccess() { - val testFile = File(projectRoot, "test.txt") - testFile.writeText("Delete me") - - val service = FileServiceImpl(projectRoot) - val result = service.deleteFile("test.txt") - - assertTrue(result.success) - assertFalse(testFile.exists()) - } - - @Test - fun testListFilesNonRecursive() { - File(projectRoot, "file1.txt").writeText("1") - File(projectRoot, "file2.txt").writeText("2") - val subdir = File(projectRoot, "subdir") - subdir.mkdirs() - File(subdir, "file3.txt").writeText("3") - - val service = FileServiceImpl(projectRoot) - val result = service.listFiles(".", false) - - assertTrue(result.success) - val files = result.data?.split("\n") ?: emptyList() - assertTrue(files.contains("file1.txt")) - assertTrue(files.contains("file2.txt")) - assertFalse(files.contains("subdir/file3.txt")) - } - - @Test - fun testListFilesRecursive() { - File(projectRoot, "file1.txt").writeText("1") - val subdir = File(projectRoot, "subdir") - subdir.mkdirs() - File(subdir, "file2.txt").writeText("2") - - val service = FileServiceImpl(projectRoot) - val result = service.listFiles(".", true) - - assertTrue(result.success) - val files = result.data?.split("\n") ?: emptyList() - assertTrue(files.contains("file1.txt")) - assertTrue(files.any { it.contains("subdir") && it.contains("file2.txt") }) - } -} diff --git a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeFileServiceImplTest.kt b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeFileServiceImplTest.kt new file mode 100644 index 0000000000..34079a1f3a --- /dev/null +++ b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeFileServiceImplTest.kt @@ -0,0 +1,116 @@ +package com.itsaky.androidide.plugins.manager.services + +import com.itsaky.androidide.plugins.PluginPermission +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import java.io.File + +class IdeFileServiceImplTest { + private lateinit var projectRoot: File + + @Before + fun setUp() { + projectRoot = + File.createTempFile("ide-file-service-", "").apply { + delete() + mkdirs() + } + } + + @After + fun tearDown() { + projectRoot.deleteRecursively() + } + + // The production allowlist only permits fixed on-device roots, so a test rooted + // at a temp dir must supply its own validator that permits paths under it. + private fun service(permissions: Set = setOf(PluginPermission.FILESYSTEM_WRITE)) = + IdeFileServiceImpl( + pluginId = "test-plugin", + permissions = permissions, + pathValidator = + object : IdeFileServiceImpl.PathValidator { + override fun isPathAllowed(path: File) = path.absolutePath.startsWith(projectRoot.absolutePath) + + override fun getAllowedPaths() = listOf(projectRoot.absolutePath) + }, + ) + + @Test + fun writeFileCreatesFileWithContent() { + val file = File(projectRoot, "new.txt") + + assertTrue(service().writeFile(file, "Content")) + assertTrue(file.exists()) + assertEquals("Content", file.readText()) + } + + @Test + fun readFileReturnsContent() { + val file = File(projectRoot, "test.txt").apply { writeText("Hello World") } + + assertEquals("Hello World", service().readFile(file)) + } + + @Test + fun readFileReturnsNullForMissingFile() { + assertNull(service().readFile(File(projectRoot, "does-not-exist.txt"))) + } + + @Test + fun replaceInFileUpdatesContent() { + val file = File(projectRoot, "test.txt").apply { writeText("Original") } + + assertTrue(service().replaceInFile(file, "Original", "Updated")) + assertEquals("Updated", file.readText()) + } + + @Test + fun deleteRemovesFile() { + val file = File(projectRoot, "test.txt").apply { writeText("Delete me") } + + assertTrue(service().delete(file)) + assertFalse(file.exists()) + } + + @Test + fun listFilesNonRecursiveReturnsDirectChildrenOnly() { + File(projectRoot, "file1.txt").writeText("1") + File(projectRoot, "file2.txt").writeText("2") + File(projectRoot, "subdir").mkdirs() + File(projectRoot, "subdir/file3.txt").writeText("3") + + val names = service().listFiles(projectRoot, recursive = false).map { it.name } + + assertTrue(names.contains("file1.txt")) + assertTrue(names.contains("file2.txt")) + assertFalse(names.contains("file3.txt")) + } + + @Test + fun listFilesRecursiveReturnsNestedChildren() { + File(projectRoot, "file1.txt").writeText("1") + File(projectRoot, "subdir").mkdirs() + File(projectRoot, "subdir/file2.txt").writeText("2") + + val names = service().listFiles(projectRoot, recursive = true).map { it.name } + + assertTrue(names.contains("file1.txt")) + assertTrue(names.contains("file2.txt")) + } + + @Test(expected = SecurityException::class) + fun disallowedPathThrowsSecurityException() { + service().readFile(File("/etc/passwd")) + } + + @Test(expected = SecurityException::class) + fun missingWritePermissionThrowsSecurityException() { + service(permissions = emptySet()).readFile(File(projectRoot, "test.txt")) + } +} diff --git a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/ResourceServiceImplTest.kt b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/ResourceServiceImplTest.kt deleted file mode 100644 index c205a64ce9..0000000000 --- a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/ResourceServiceImplTest.kt +++ /dev/null @@ -1,42 +0,0 @@ -package com.itsaky.androidide.plugins.manager.services - -import com.itsaky.androidide.plugins.services.IdeResourceService -import org.junit.Test -import org.junit.Assert.* -import java.io.File - -class ResourceServiceImplTest { - - @Test - fun testGetStringSuccess() { - val projectRoot = File.createTempFile("project", "").parentFile!! - val service = ResourceServiceImpl(projectRoot) - - val result = service.getString("app_name") - - assertTrue(result.success) - assertNotNull(result.data) - } - - @Test - fun testGetDrawableSuccess() { - val projectRoot = File.createTempFile("project", "").parentFile!! - val service = ResourceServiceImpl(projectRoot) - - val result = service.getDrawable("ic_launcher") - - assertTrue(result.success) - assertNotNull(result.data) - } - - @Test - fun testGetColorSuccess() { - val projectRoot = File.createTempFile("project", "").parentFile!! - val service = ResourceServiceImpl(projectRoot) - - val result = service.getColor("primary_color") - - assertTrue(result.success) - assertNotNull(result.data) - } -} From 60f150d845d08c023c22e19ff5d83c341d393c0e Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Thu, 30 Jul 2026 11:53:59 -0700 Subject: [PATCH 2/3] ADFA-4936: Dispose per-test Kotlin Analysis-API environment to stop OOM The nightly's second failure was :lsp:kotlin:testV8DebugUnitTest exceeding its 10-minute per-task timeout. Root cause: KtLspTestRule built a heavy KtLspTestEnvironment (IntelliJ project, application env, FIR infra, background index workers) per test method but env.close() was disabled, so environments accumulated for the JVM's lifetime. Locally this reproduces as java.lang.OutOfMemoryError after ~130 tests; in CI (larger heap) it manifests as GC-thrash plus a global-analysis-lock waiter crawling past the 10-minute timeout. Either way the run fails and the aggregate report is skipped. - KtLspTestRule: re-enable env.close() in the finally block. The original "fails in test cases" was caused by calling it inside project.write { }; close() manages its own threading/locking, so call it directly. - AnalysisSerializationTest: add @Test(timeout = 60_000) to the two heavy concurrency tests that previously had no timeout, so a genuine liveness stall fails fast instead of burning the task budget (defense in depth; ~130x their observed runtime). No production code changed. Verified: :lsp:kotlin:testV8DebugUnitTest -> 169 tests, 0 failures (previously OOM/timeout). --- .../modules/AnalysisSerializationTest.kt | 4 +- .../lsp/kotlin/fixtures/KtLspTestRule.kt | 40 ++++++++++--------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt index fc96fc014b..72d55ce4d2 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/compiler/modules/AnalysisSerializationTest.kt @@ -38,7 +38,7 @@ import kotlin.math.max * preemption, and reentrancy behaviour. */ class AnalysisSerializationTest : KtLspTest() { - @Test + @Test(timeout = 60_000) fun `concurrent analyzeMaybeDangling never throws lifetime exception`(): Unit = runBlocking { val files = @@ -86,7 +86,7 @@ class AnalysisSerializationTest : KtLspTest() { assertThat(errors).isEmpty() } - @Test + @Test(timeout = 60_000) fun `analyzeMaybeDangling serializes overlapping analyses`(): Unit = runBlocking { val files = diff --git a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestRule.kt b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestRule.kt index 34fe125673..00ca7b8159 100644 --- a/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestRule.kt +++ b/lsp/kotlin/src/test/java/com/itsaky/androidide/lsp/kotlin/fixtures/KtLspTestRule.kt @@ -7,31 +7,35 @@ import org.junit.runner.Description import org.junit.runners.model.Statement internal class KtLspTestRule : TestRule { - val tempDir = TemporaryFolder() lateinit var env: KtLspTestEnvironment private set override fun apply( statement: Statement?, - description: Description? - ): Statement { - return tempDir.apply(object : Statement() { - override fun evaluate() { - try { - val sourceRoot = tempDir.newFolder("src").toPath() - env = KtLspTestEnvironment(listOf(sourceRoot)) + description: Description?, + ): Statement = + tempDir.apply( + object : Statement() { + override fun evaluate() { + try { + val sourceRoot = tempDir.newFolder("src").toPath() + env = KtLspTestEnvironment(listOf(sourceRoot)) - statement?.evaluate() - } finally { - if (::env.isInitialized) { - env.project.write { - // TODO: This fails in test cases, ignored for now - // env.close() + statement?.evaluate() + } finally { + if (::env.isInitialized) { + // Dispose each test's heavy Analysis-API environment (IntelliJ project, + // application env, background index workers). Without this the per-method + // environments accumulate for the JVM's lifetime and the suite eventually + // exhausts the heap (ADFA-4936). close() manages its own threading/locking, + // so it must NOT run inside project.write { } -- that nesting was the + // original "fails in test cases" failure. + env.close() } } } - } - }, description) - } -} \ No newline at end of file + }, + description, + ) +} From bee1a0fd21d8f17a4e333566f235d8e45af52eec Mon Sep 17 00:00:00 2001 From: Hal Eisen Date: Thu, 30 Jul 2026 14:01:01 -0700 Subject: [PATCH 3/3] ADFA-4936: Use canonical path containment in plugin-manager test validators Review feedback. The test PathValidator fakes used path.absolutePath.startsWith(root.absolutePath), which false-accepts a sibling-prefix path (e.g. "-sibling") and an unnormalized traversal ("/../escape"). Replace with canonical, component-wise containment (canonicalFile.toPath().startsWith(root.canonicalFile.toPath())) via a small File.isWithin helper, and add IdeFileServiceImplTest coverage rejecting both sibling-prefix and traversal paths (within-root acceptance stays covered by the existing read/write tests). Verified: :plugin-manager:testV8DebugUnitTest -> 15 tests, 0 failures; spotlessCheck clean. --- .../plugins/manager/E2EIntegrationTest.kt | 6 +++++- .../core/PluginManagerIntegrationTest.kt | 6 +++++- .../services/IdeFileServiceImplTest.kt | 21 ++++++++++++++++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/E2EIntegrationTest.kt b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/E2EIntegrationTest.kt index 9fbd8e7608..eaca5ab1f8 100644 --- a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/E2EIntegrationTest.kt +++ b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/E2EIntegrationTest.kt @@ -39,7 +39,7 @@ class E2EIntegrationTest { permissions = setOf(PluginPermission.FILESYSTEM_WRITE), pathValidator = object : IdeFileServiceImpl.PathValidator { - override fun isPathAllowed(path: File) = path.absolutePath.startsWith(tempProjectRoot.absolutePath) + override fun isPathAllowed(path: File) = path.isWithin(tempProjectRoot) override fun getAllowedPaths() = listOf(tempProjectRoot.absolutePath) }, @@ -61,4 +61,8 @@ class E2EIntegrationTest { assertTrue("delete should succeed", resolved.delete(sourceFile)) assertFalse("deleted file should be gone", sourceFile.exists()) } + + // Canonical, component-wise containment: rejects sibling-prefix and traversal escapes that a + // naive absolutePath.startsWith() would wrongly accept. + private fun File.isWithin(root: File) = canonicalFile.toPath().startsWith(root.canonicalFile.toPath()) } diff --git a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManagerIntegrationTest.kt b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManagerIntegrationTest.kt index 61f6a3e52e..67f50c90ec 100644 --- a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManagerIntegrationTest.kt +++ b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManagerIntegrationTest.kt @@ -101,7 +101,7 @@ class PluginManagerIntegrationTest { permissions = setOf(PluginPermission.FILESYSTEM_WRITE), pathValidator = object : IdeFileServiceImpl.PathValidator { - override fun isPathAllowed(path: File) = path.absolutePath.startsWith(tempProjectRoot.absolutePath) + override fun isPathAllowed(path: File) = path.isWithin(tempProjectRoot) override fun getAllowedPaths() = listOf(tempProjectRoot.absolutePath) }, @@ -114,4 +114,8 @@ class PluginManagerIntegrationTest { override fun getProjectByPath(path: File): IProject? = null } + + // Canonical, component-wise containment: rejects sibling-prefix and traversal escapes that a + // naive absolutePath.startsWith() would wrongly accept. + private fun File.isWithin(root: File) = canonicalFile.toPath().startsWith(root.canonicalFile.toPath()) } diff --git a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeFileServiceImplTest.kt b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeFileServiceImplTest.kt index 34079a1f3a..a87086cac0 100644 --- a/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeFileServiceImplTest.kt +++ b/plugin-manager/src/test/kotlin/com/itsaky/androidide/plugins/manager/services/IdeFileServiceImplTest.kt @@ -35,7 +35,7 @@ class IdeFileServiceImplTest { permissions = permissions, pathValidator = object : IdeFileServiceImpl.PathValidator { - override fun isPathAllowed(path: File) = path.absolutePath.startsWith(projectRoot.absolutePath) + override fun isPathAllowed(path: File) = path.isWithin(projectRoot) override fun getAllowedPaths() = listOf(projectRoot.absolutePath) }, @@ -109,8 +109,27 @@ class IdeFileServiceImplTest { service().readFile(File("/etc/passwd")) } + @Test(expected = SecurityException::class) + fun siblingPrefixPathThrowsSecurityException() { + // A sibling dir whose path is a string prefix of the root ("-sibling") must be + // rejected -- naive absolutePath.startsWith() would wrongly accept it. + val sibling = File(projectRoot.parentFile, "${projectRoot.name}-sibling") + service().readFile(File(sibling, "secret.txt")) + } + + @Test(expected = SecurityException::class) + fun traversalPathThrowsSecurityException() { + // A traversal that escapes the root ("/../escape.txt") must be rejected -- naive + // absolutePath.startsWith() would wrongly accept it since ".." is not normalized. + service().readFile(File(projectRoot, "../escape.txt")) + } + @Test(expected = SecurityException::class) fun missingWritePermissionThrowsSecurityException() { service(permissions = emptySet()).readFile(File(projectRoot, "test.txt")) } + + // Canonical, component-wise containment: rejects sibling-prefix and traversal escapes that a + // naive absolutePath.startsWith() would wrongly accept. + private fun File.isWithin(root: File) = canonicalFile.toPath().startsWith(root.canonicalFile.toPath()) }