fix: prevent test contamination by cleaning up untracked files by djessup · Pull Request #15 · djessup/idea-worktrees

djessup · 2025-10-21T04:46:00Z

Description

Fixes #14

This PR resolves test contamination where testCompareWorktreesDetectsChanges fails when run as part of the full test suite but passes in isolation.

Root Cause

Tests in GitWorktreeServiceTest share the same project directory. When tests create files (e.g., base.txt and main.txt in testMergeWorktreeFastForwardFailureOnDivergedHistory), these files remain as untracked files for subsequent tests. The compareWorktrees operation checks for uncommitted changes using git status --porcelain and fails if any untracked files are present.

Solution

Added a cleanupUntrackedFiles() method to AbstractGitWorktreeTestCase that:

Runs in tearDown() after each test
Identifies untracked files using git status --porcelain
Deletes untracked files (except .gitignore)
Handles cases where .git directory doesn't exist
Silently ignores cleanup errors to prevent test failures

Testing

✅ All tests pass when run individually
✅ All tests pass when run as a full suite with --rerun-tasks
✅ Verified multiple consecutive runs are stable
✅ Code coverage remains above 80% threshold

Changes

Modified AbstractGitWorktreeTestCase.tearDown() to call cleanup
Added cleanupUntrackedFiles() private method
No changes to production code

- Add WorktreeNotifications utility for centralized notification management - Add WorktreeResultHandler utility for standardized result handling - Add WorktreeAsyncOperations utility for async operations with EDT threading - Add comprehensive tests for all three utilities with >85% coverage - All utilities are thread-safe and properly marshal callbacks to EDT Refs: SPEC-001 Phase 1 Tasks 1-3

…(SPEC-001) - Replace listWorktrees().thenCombine() pattern with WorktreeAsyncOperations.loadWorktreesWithCurrent() - Replace direct notification calls with WorktreeNotifications.showError() - Remove ApplicationManager and ModalityState boilerplate - Simplify error handling with centralized utilities Refs: SPEC-001 Phase 2 Task 6

…(SPEC-001) - Replace listWorktrees().thenCombine() pattern with WorktreeAsyncOperations.loadWorktreesWithCurrent() - Replace direct notification calls with WorktreeNotifications.showError() - Replace result handling logic with WorktreeResultHandler.handle() - Remove ApplicationManager and ModalityState boilerplate - Simplify error handling with centralized utilities Refs: SPEC-001 Phase 2 Task 7

…(SPEC-001) - Replace listWorktrees() pattern with WorktreeAsyncOperations.loadWorktrees() - Replace result handling logic with WorktreeResultHandler.handle() - Remove duplicate notify() helper method - Remove ApplicationManager and ModalityState boilerplate - Simplify error handling with centralized utilities Refs: SPEC-001 Phase 2 Task 8

… (SPEC-001) - Replace listWorktrees() pattern with WorktreeAsyncOperations.loadWorktrees() - Remove ApplicationManager and ModalityState boilerplate - Simplify async operation handling with centralized utilities Refs: SPEC-001 Phase 2 Task 9

…SPEC-001) - Replace listWorktrees() pattern with WorktreeAsyncOperations.loadWorktrees() - Replace result handling logic with WorktreeResultHandler.handle() - Remove duplicate notify() helper method - Remove ApplicationManager and ModalityState boilerplate - Simplify error handling with centralized utilities Refs: SPEC-001 Phase 2 Task 10

…(SPEC-001) - Replace result handling logic with WorktreeResultHandler.handle() - Use promptToOpen parameter for worktree opening prompt - Remove ApplicationManager and ModalityState boilerplate - Remove direct notification calls - Simplify error handling with centralized utilities Refs: SPEC-001 Phase 2 Task 11

…C-001) - Replace listWorktrees/getCurrentWorktree pattern with WorktreeAsyncOperations.loadWorktreesWithCurrent() - Replace result handling logic with WorktreeResultHandler.handle() - Replace direct notification calls with WorktreeNotifications.showError() - Remove ApplicationManager and ModalityState boilerplate - Simplify error handling with centralized utilities Refs: SPEC-001 Phase 3 Task 12

…PEC-001) - Replace listWorktrees/getCurrentWorktree pattern with WorktreeAsyncOperations.loadWorktreesWithCurrent() - Replace result handling logic with WorktreeResultHandler.handle() - Replace direct notification calls with WorktreeNotifications.showError() - Remove ApplicationManager and ModalityState boilerplate - Simplify error handling with centralized utilities Refs: SPEC-001 Phase 3 Task 13

- Updated AGENTS.md with refactoring completion notes - Added utility classes to Architecture Snapshot - Documented code duplication reduction (~250+ lines) - Updated tasks.md with completion status - All 26 new tests passing - 7 action classes and 2 UI components migrated SPEC-001

All 17 tasks in SPEC-001 code consolidation are now complete: - Phase 1: Created 3 utility classes with 26 tests - Phase 2: Migrated 7 action classes - Phase 3: Migrated 2 UI components - Phase 4: Verified cleanup and updated documentation SPEC-001

…and ignoring IDE artifacts\n\n- Clean up case-insensitive path test to remove created files\n- Add .gitignore in test fixture and stage it to avoid IDE/test noise being treated as dirty\n\nVerifies: full suite runs reliably; passes in isolation and in suite

- Added cleanupUntrackedFiles() method to AbstractGitWorktreeTestCase - Cleanup runs in tearDown() to remove untracked files between tests - Prevents files from one test affecting subsequent tests - Handles cases where .git directory doesn't exist (e.g., testListWorktreesWhenRepositoryMissingGitDir) Fixes #14

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2025-10-21T04:49:15Z

src/main/kotlin/au/id/deejay/ideaworktrees/actions/CompareWorktreeAction.kt

+                                DiffResultDialog(project, message, details).show()
                            } else {
-                                val message = result.errorMessage() ?: "Failed to compare worktrees."
-                                val details = result.errorDetails()
-                                val fullMessage = if (details != null) "$message\n\nDetails: $details" else message
-                                Messages.showErrorDialog(project, fullMessage, "Compare Worktrees")
+                                Messages.showInfoMessage(project, message, "Compare Worktrees")
                            }
-                        }, ModalityState.nonModal())
+                        } else {


Show comparison dialog on background thread

The completion handler of service.compareWorktrees now shows DiffResultDialog and Messages.show* directly (around these lines) without scheduling the block onto the EDT. CompletableFuture.whenComplete executes on the thread that finishes the task, so when the Git call completes off the EDT these UI calls will trigger assertIsDispatchThread failures or hang the UI. Wrap the handler in ApplicationManager.invokeLater or use an EDT-aware helper before interacting with Swing components.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2025-10-21T04:49:15Z

src/main/kotlin/au/id/deejay/ideaworktrees/actions/CreateWorktreeAction.kt

                is WorktreeOperationResult.RequiresInitialCommit -> {
                    val response = Messages.showYesNoDialog(


Initial-commit prompt runs off EDT in create action

In the RequiresInitialCommit branch the code calls Messages.showYesNoDialog/showInfoMessage directly inside the createWorktree().whenComplete callback (after removing the previous invokeLater). When the future completes on a background thread these dialogs will execute outside the EDT, violating IntelliJ threading rules and causing runtime assertions. Marshal to the EDT before displaying the dialog.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2025-10-21T04:49:16Z

src/main/kotlin/au/id/deejay/ideaworktrees/ui/WorktreeStatusBarWidget.kt

                    is WorktreeOperationResult.RequiresInitialCommit -> {
                        val response = Messages.showYesNoDialog(


Status bar initial-commit dialog not marshalled to EDT

The status bar widget’s create-worktree flow similarly shows Messages.showYesNoDialog when a repository lacks an initial commit, but this handler is invoked from service.createWorktree(...).whenComplete after the surrounding invokeLater was removed. The future may complete on a pooled thread, so the dialog and follow-up info message will run off the EDT and can throw AssertionError: Must be invoked on EDT. Schedule these UI prompts onto the EDT (e.g., via ApplicationManager.invokeLater).

Useful? React with 👍 / 👎.

@codex review

…\n- CompareWorktreeAction: wrap result handler in ApplicationManager.invokeLater to show DiffResultDialog/Messages on EDT\n- CreateWorktreeAction: schedule initial-commit prompt to EDT inside whenComplete handler\n- WorktreeStatusBarWidget: schedule initial-commit prompt/info to EDT in create-worktree flow\n\nAddresses PR #15 review comments about off-EDT UI calls

djessup · 2025-10-21T11:22:12Z

@codex review fixes

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2025-10-21T11:25:20Z

src/main/kotlin/au/id/deejay/ideaworktrees/ui/ManageWorktreesDialog.kt

+        service.deleteWorktree(worktree.path, force).whenComplete { result, error ->
+            if (error != null) {
+                WorktreeNotifications.showError(
+                    project = project,
+                    title = "Error Deleting Worktree",
+                    message = error.message ?: "Unknown error occurred while deleting worktree"
+                )
+                return@whenComplete
            }
+
+            if (result != null) {
+                WorktreeResultHandler.handle(
+                    project = project,
+                    result = result,
+                    successTitle = "Worktree Deleted",
+                    errorTitle = "Error Deleting Worktree"
+                )
+                if (result.isSuccess) {
+                    refreshWorktrees()


Marshal refreshWorktrees() to EDT after deletion

The whenComplete callback of service.deleteWorktree executes on the background thread that completed the future. Inside that callback we call refreshWorktrees(), which immediately mutates the worktrees list, fires table model events, and updates Swing buttons. These operations must run on the EDT, otherwise deletion can refresh the table off the UI thread and intermittently throw assertion errors or render stale state. Schedule the refresh on the EDT (e.g. via invokeLater) before touching Swing components.

Useful? React with 👍 / 👎.

David Jessup added 16 commits October 20, 2025 09:06

docs: mark SPEC-001 tasks as complete

6fa2bf4

All 17 tasks in SPEC-001 code consolidation are now complete: - Phase 1: Created 3 utility classes with 26 tests - Phase 2: Migrated 7 action classes - Phase 3: Migrated 2 UI components - Phase 4: Verified cleanup and updated documentation SPEC-001

fix: restore EDT marshalling in result handler

a84c57f

chore: remove .intellijplatform

87fe95b

chore: ignore .intellijplatform

ab6f2da

chatgpt-codex-connector bot reviewed Oct 21, 2025

View reviewed changes

djessup and others added 6 commits October 21, 2025 19:57

Delete .kiro/steering/product.md

a6bad3d

Delete .kiro/steering/structure.md

55dafd7

Delete .kiro/steering/tech.md

1107ff6

Delete .kiro/hooks/docs-sync-hook.kiro.hook

0f00a3e

chore: ignore IDE VCS mapping and remove from PR (#15)

1472b2b

djessup self-assigned this Oct 21, 2025

djessup added the bug Something isn't working label Oct 21, 2025

chatgpt-codex-connector bot reviewed Oct 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: prevent test contamination by cleaning up untracked files#15

fix: prevent test contamination by cleaning up untracked files#15
djessup wants to merge 22 commits intomasterfrom
fix-issue-14-test-contamination

djessup commented Oct 21, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Oct 21, 2025

Uh oh!

chatgpt-codex-connector bot Oct 21, 2025

Uh oh!

chatgpt-codex-connector bot Oct 21, 2025

Uh oh!

djessup Oct 21, 2025

Uh oh!

djessup commented Oct 21, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Oct 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		is WorktreeOperationResult.RequiresInitialCommit -> {
		val response = Messages.showYesNoDialog(

Conversation

djessup commented Oct 21, 2025

Description

Root Cause

Solution

Testing

Changes

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

djessup Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

djessup commented Oct 21, 2025

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant