Visual improvements to worker lines with live test-progress suffix (2.x)

[ammachado]

What

Presentation-only reshaping of mvnd's live terminal display, plus the client-side plumbing for a live per-test progress suffix, on master (2.x, Maven 4).

Live now (driven by existing messages)

• A drawn progress bar with percent prefixed to the status line.
• Concise arrow-style worker lines: > :module goal (execution).
• Dimmed > IDLE slots for free worker threads.

Wired but dormant (client-side only)

• New PROJECT_TEST_PROGRESS wire message (Message.ProjectTestProgressEvent, round-trip serialization).
• TerminalOutput.appendTestProgress(...) renders a styled suffix [Tests: N, Failures: .., Errors: .., Skipped: ..] Class#method, with failure/error counts in red.

Follow-up

The daemon-side feed that emits PROJECT_TEST_PROGRESS (the surefire ForkNodeFactory bridge) is not implemented on master yet, so the test-progress suffix stays dormant until a follow-up commit lands it. This is a deliberate, spike-gated separation; a TODO on the PROJECT_TEST_PROGRESS constant records it.

Verification

• common unit tests pass (message round-trip; bar geometry; suffix formatting and styles), RAT clean.
• MultiModuleTest integration test passes (the reshaping did not break message-driven client assertions).
• Built and verified with JDK 17.

🤖 Generated with Cursor

[gnodet]

Well-structured UI improvement with solid tests. Two design points to discuss before the daemon-side follow-up lands:

1. Message priority ordering (medium): PROJECT_TEST_PROGRESS is assigned class order 2 in getClassOrder(), which is higher priority than PROJECT_STARTED (3) and MOJO_STARTED (4). Since the daemon's sendQueue is a PriorityBlockingQueue ordered by getClassOrder(), a test progress message could theoretically be dequeued before the corresponding PROJECT_STARTED, causing projects.get(e.getProjectId()) to return null and silently drop the message. Practical risk is low since the daemon emitter is not yet implemented, but consider placing it at order 5 or higher before the follow-up — the fix is a one-line change.

2. Removed plugin version from display (medium): The worker line format changed from prefix:version:mojo to prefix:mojo, removing the version that was deliberately added in commit 2a8d18d (PR #748). The groupId fallback when no goalPrefix is set also dropped the groupId. This appears to be an intentional UX tradeoff for conciseness by the same author, but it does lose debugging info for plugin version conflicts. Worth confirming this is the desired direction.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

[gnodet]

Well-structured presentation improvement PR with good test coverage. The concise arrow-style worker lines, idle-slot rendering, and progress bar are all nicely implemented.

One item to consider:

Redundant writeNullableUTF / readNullableUTF (medium)

The new helper methods introduce a second wire-format convention for nullable strings (boolean prefix) when the existing writeUTF/readUTF already handle null via the -1 sentinel convention. Every other nullable string in Message.java uses writeUTF/readUTF directly.

The ProjectTestProgressEvent serialization can simply use the existing methods:

// read:
final String testClass = readUTF(input);
final String testMethod = readUTF(input);

// write:
writeUTF(output, testClass);
writeUTF(output, testMethod);

This avoids maintaining two incompatible null-encoding conventions in the same class and saves 1 byte per nullable field on the wire.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

[gnodet]

Re-review after rebase (d85c8af → c716d57): no functional changes detected.

The previous finding about redundant writeNullableUTF/readNullableUTF remains unaddressed — the existing writeUTF/readUTF already handle null via the -1 sentinel convention. ProjectTestProgressEvent can simply use them directly, avoiding a second incompatible wire-format convention in the same class.

Visual improvements and test coverage continue to look good.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

[ammachado]

Once #1670 is merged, I'll port it to the master branch.

[gnodet]

Re-review after new commit (786ec64): the writeNullableUTF/readNullableUTF finding from the previous review has been fully addressed — the redundant helpers are removed and the existing readUTF/writeUTF null-sentinel convention is used correctly. The null round-trip test covers this path. No new issues introduced.

Minor note (not blocking): PROJECT_TEST_PROGRESS still sits at class order 2, higher priority than PROJECT_STARTED (3) and MOJO_STARTED (4). When the daemon-side emitter lands (follow-up PR), a test-progress message could be dequeued before its corresponding PROJECT_STARTED. Consider bumping it to order 5+ before that follow-up merges.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of @gnodet

[gnodet]

Re-review after commit 7bb1417: both prior findings are now resolved.

1. Message priority ordering — PROJECT_TEST_PROGRESS moved from class order 2 to 5 (after MOJO_STARTED=4), preventing the dequeue race.
2. Plugin version display — pluginGroupId and pluginVersion restored in the worker line, matching master.

No new issues. Looks good! ✅

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of @gnodet

[github-actions]

@slachiewicz Please assign appropriate label to PR according to the type of change.