Add real Cleanup previews and atomic coordinated candidate UI - #1464
Conversation
Co-authored-by: carstenartur <3164220+carstenartur@users.noreply.github.com>
Co-authored-by: carstenartur <3164220+carstenartur@users.noreply.github.com>
JUnit Test Overview ReportGenerated on: 31804077934 Overall Statistics
Test Summary by Plugin
Disabled Tests DetailsNo disabled tests found! 🎉 Generated by the Test Source Inventory workflow |
|
The stale-main conflict chain is resolved on head
GitHub consequently recognized #1466 as merged and closed it automatically. This PR is now mergeable; fresh Maven, distribution, Eclipse Help, CodeQL, and Codacy checks are running on the reconciled head. |
|
The exact current head |
7d9910d to
77aaca5
Compare
490433e to
ab9cc16
Compare
Bring the three current workflow action updates into the feature branch before final preview verification.
|
The final feature head is now Changes made after inspecting the failed SWTBot diagnostics:
The previous failing run did not report a Maven/product-build failure or a |
|
The current head limits the default Help screenshot run to the five stable SWTBot scenarios so this reviewable feature slice can be completed without growing further. Two Java/JUnit/SWTBot methods remain to be repaired under #1437:
Their successful execution is not asserted by this PR. They remain present and will return to the default Maven/Tycho profile after their fixture synchronization is fixed. The ordinary Maven/JUnit suite and the other screenshot scenarios continue to run. |
There was a problem hiding this comment.
Pull request overview
This PR strengthens Sandbox’s Cleanup UX and documentation by (1) adding SWTBot coverage for the real LTK execution preview (not just the preferences-tab sample preview) and (2) introducing a dedicated preview viewer + metadata contract for coordinated multi-file candidates so they can be presented as atomic selection units.
Changes:
- Adds deterministic SWTBot scenarios that open Source > Clean Up..., assert preview tree labels/diff changes, apply selective edits, and verify undo.
- Introduces a coordinated multi-file preview contract (
getCoordinatedCleanUpPreview) plus anIChangePreviewViewerthat shows candidate metadata and per-file read-only diffs without exposing unsafe child checkboxes. - Updates Help pages and structural checks (including
/topic/...link validation), plus workflow/tooling updates for screenshot runs and the headless Cleanup application ID.
Reviewed changes
Copilot reviewed 35 out of 38 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sandbox_usage_view_test/src/org/sandbox/jdt/ui/helper/views/SandboxHelpScreenshotsSWTBotTest.java | Adds real Clean Up wizard preview capture/verification, fixture project creation, and improved dialog handling. |
| sandbox_usage_view_test/src/org/sandbox/jdt/ui/helper/views/SandboxHelpScreenshotsMergeGateSWTBotTest.java | Adds merge-gate wrapper test to run a subset of screenshot captures in CI. |
| sandbox_usage_view_test/src/org/sandbox/jdt/ui/helper/views/EclipseHelpStructureTest.java | Extends Help structural validation to include new screenshots and validate /topic/... references. |
| sandbox_usage_view_test/pom.xml | Increases SWTBot runtime tolerance (timeouts) for CI stability. |
| sandbox_junit_cleanup_help/html/usage.html | Links to the shared “Cleanup preview model” Help page. |
| sandbox_jface_cleanup/src/org/sandbox/jdt/internal/ui/preferences/cleanup/SandboxCodeTabPage.java | Exposes and wires a JFace master cleanup option and slaves in the preferences UI. |
| sandbox_jface_cleanup/src/org/sandbox/jdt/internal/ui/fix/MultiFixMessages.properties | Adds user-facing per-migration step description strings for JFace cleanups. |
| sandbox_jface_cleanup/src/org/sandbox/jdt/internal/ui/fix/MultiFixMessages.java | Declares new NLS fields for JFace step descriptions. |
| sandbox_jface_cleanup/src/org/sandbox/jdt/internal/ui/fix/JFaceCleanUpCore.java | Changes step descriptions to emit one user-facing description per enabled migration. |
| sandbox_jface_cleanup/src/org/sandbox/jdt/internal/corext/fix/JfaceCleanUpFixCore.java | Provides lazy, NLS-backed per-migration step descriptions and uses them for TextEditGroup labels. |
| sandbox_jface_cleanup_test/src/org/sandbox/jdt/ui/tests/quickfix/JFaceCleanUpDescriptionsTest.java | Adds regression tests for the new step-description behavior and master gating. |
| sandbox_jface_cleanup_help/html/usage.html | Documents real execution preview behavior and embeds new canonical screenshots. |
| sandbox_int_to_enum_help/html/usage.html | Links to the shared “Cleanup preview model” Help page. |
| sandbox_functional_converter/src/org/sandbox/jdt/internal/ui/preferences/cleanup/CleanUpMessages.properties | Updates/realigns NLS properties (includes additional keys). |
| sandbox_common/src/org/sandbox/jdt/cleanup/multifile/ui/CoordinatedCleanUpPreviewViewer.java | Adds a dedicated LTK change preview viewer for coordinated multi-file cleanup candidates. |
| sandbox_common/src/org/sandbox/jdt/cleanup/multifile/AbstractPlannedMultiFileCleanUp.java | Adds public, dependency-light coordinated-preview metadata contract with fail-closed validation. |
| sandbox_common/plugin.xml | Registers the coordinated preview viewer via org.eclipse.ltk.ui.refactoring.changePreviewViewers. |
| sandbox_common/META-INF/MANIFEST.MF | Adds required bundles for LTK UI refactoring and Compare integration. |
| sandbox_common_test/src/org/sandbox/jdt/cleanup/multifile/CoordinatedCleanUpPreviewContractTest.java | Adds unit tests for the coordinated preview metadata contract and fail-closed behavior. |
| sandbox_cleanup_application/src/org/sandbox/jdt/core/cleanupapp/messages.properties | Fixes command-line usage text to the registered Equinox application ID. |
| sandbox_cleanup_application/README.md | Updates docs to use the corrected application ID. |
| sandbox_cleanup_application/ARCHITECTURE.md | Updates docs to use the corrected application ID. |
| sandbox_cleanup_application_help/toc.xml | Adds “Cleanup preview model” to the Help TOC. |
| sandbox_cleanup_application_help/html/usage.html | Updates Help to use the corrected application ID. |
| sandbox_cleanup_application_help/html/index.html | Updates Help to use the corrected application ID and links shared IDE guidance. |
| sandbox_cleanup_application_help/html/cleanup-preview.html | Adds shared Help page describing config preview vs real execution preview and atomic coordinated migrations. |
| .github/workflows/test-report.yml | Updates GitHub Actions versions (Python + github-script). |
| .github/workflows/publish-cleanup-image.yml | Updates application ID in generated script and bumps buildx action version. |
| .github/workflows/pr-auto-cleanup.yml | Bumps github-script action version. |
| .github/workflows/post-merge-provenance.yml | Updates setup-python action version. |
| .github/workflows/maven.yml | Updates setup-node action version. |
| .github/workflows/eclipse-help-screenshots.yml | Adds an outer timeout, runs merge-gate SWTBot class, and restores deferred real-preview images. |
| .github/workflows/coverage.yml | Updates setup-node action version. |
| .github/workflows/capability-inventory.yml | Updates setup-python action version. |
| .github/actions/cleanup-action/action.yml | Updates application ID used by the cleanup action runner. |
Suppressed comments (1)
sandbox_common/src/org/sandbox/jdt/cleanup/multifile/ui/CoordinatedCleanUpPreviewViewer.java:187
getChanges()is called reflectively and may legally returnnull(or be absent and throw), but the code unconditionally iteratesfor (Change child : changes), which will throw an NPE if the metadata returnsnull. Treat a null array as empty so the viewer can still render the header/details cleanly.
String candidateDescription= invoke(change, "getDescription", String.class); //$NON-NLS-1$
List<String> safetyDetails= stringList(invoke(change, "getSafetyDetails", List.class)); //$NON-NLS-1$
Change[] changes= invoke(change, "getChanges", Change[].class); //$NON-NLS-1$
List<FilePreview> filePreviews= new ArrayList<>();
for (Change child : changes) {
collectFilePreviews(child, filePreviews);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Addressed the remaining review note in |
Preserve the real cleanup preview and Help infrastructure already merged on main while retaining the JUnit migration implementation and its dedicated QA. Resolve unrelated screenshot baselines in favor of main.
Purpose
Deliver the coordinated Cleanup preview UI as one reviewable slice. Ordinary local edits keep Eclipse's fine-grained selection; planned multi-file migrations appear as one atomic candidate.
Implemented
AbstractPlannedMultiFileCleanUp;org.sandbox.jdt.core.JavaCleanup;Temporary test boundary
The SWTBot class still contains the real independent-selection and atomic Int-to-Enum scenarios. Those two scenarios are tracked by #1437 and are temporarily outside the merge gate; they are not deleted or reported as passing.
The merge gate executes the five stable Help scenarios with Maven/Tycho and explicitly requires a JUnit XML report for
SandboxHelpScreenshotsMergeGateSWTBotTest.This PR stays below the agreed review-size threshold and does not absorb #1467 or #1468.
Refs #1451
Refs #1452
Refs #1437
Refs carstenartur/eclipse.jdt.ui#100