Skip to content

Tighten parity gate and retire CLI language leaks#1637

Merged
TSavo merged 3 commits into
mainfrom
codex/testng-emitter-parity-gate
May 29, 2026
Merged

Tighten parity gate and retire CLI language leaks#1637
TSavo merged 3 commits into
mainfrom
codex/testng-emitter-parity-gate

Conversation

@TSavo
Copy link
Copy Markdown
Owner

@TSavo TSavo commented May 29, 2026

Summary

  • add the Java TestNG emitter dispatch test to make cross-language-proof-parity
  • remove the Go/testing artifact-path special case from the Rust CLI emit fallback
  • retire the legacy provekit link subcommand, which hard-coded Rust/Go project layout and Go source/LSP handling in the CLI

Verification

  • rg -n "cmd_emit_java_testng" Makefile
  • make -n cross-language-proof-parity | rg -n "cmd_emit_java_testng|emit_java_testng_dispatches_real_emitter"
  • cargo test --release --manifest-path implementations/rust/Cargo.toml -p provekit-cli --test cmd_emit_java_testng emit_java_testng_dispatches_real_emitter_and_maven_checks_output -- --nocapture
  • red: cargo test --manifest-path implementations/rust/Cargo.toml -p provekit-cli default_artifact_path_is_not_language_framework_special_cased -- --nocapture failed with CLI returning provekit_emitted_test.go for Go/testing
  • green: cargo test --manifest-path implementations/rust/Cargo.toml -p provekit-cli default_artifact_path_is_not_language_framework_special_cased -- --nocapture
  • cargo test --release --manifest-path implementations/rust/Cargo.toml -p provekit-cli --test cmd_emit_go_testing emit_go_testing_dispatches_manifest_writes_artifact_and_compile_checks -- --nocapture
  • red: cargo test --manifest-path implementations/rust/Cargo.toml -p provekit-cli --test cli_surface provekit_cli_does_not_expose_legacy_link_subcommand -- --nocapture failed while link was exposed
  • green: cargo test --manifest-path implementations/rust/Cargo.toml -p provekit-cli --test cli_surface provekit_cli_does_not_expose_legacy_link_subcommand -- --nocapture
  • cargo test --manifest-path implementations/rust/Cargo.toml -p provekit-cli --test cli_surface -- --nocapture

Summary by CodeRabbit

  • Chores

    • Removed the provekit link command from the CLI.
  • Bug Fixes

    • Standardized Go emit output naming behavior to consistently use the JSON extension.
  • Tests

    • Extended parity testing to include Java TestNG emitter coverage.
    • Added verification that the link command is no longer accessible.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

Walkthrough

The PR removes the legacy provekit link CLI subcommand entirely by deleting cmd_link.rs and unwiring its entry point from main.rs, verified by a new CLI surface test. It also simplifies artifact path defaults by removing go+testing special-casing and extends Java emit parity testing to include TestNG.

Changes

Remove legacy link subcommand

Layer / File(s) Summary
CLI subcommand removal
implementations/rust/provekit-cli/src/main.rs
Removes the cmd_link module declaration, deletes the Cmd::Link(LinkArgs) enum variant, removes the public LinkArgs struct definition, and removes the Cmd::Link dispatch handler from main().
Verification test
implementations/rust/provekit-cli/tests/cli_surface.rs
New provekit_cli_does_not_expose_legacy_link_subcommand test verifies the link subcommand does not appear in --help and that invoking provekit link project is rejected with a clap-style error.

Emit artifact path normalization

Layer / File(s) Summary
Remove go+testing special-case
implementations/rust/provekit-cli/src/cmd_emit.rs
default_artifact_path signature changes to ignore the framework parameter; output is now always derived from the JSON extension field (or falls back to target). New unit test confirms default_artifact_path("go", "testing", &json!({})) returns provekit_emitted.go without special-casing.
Add TestNG to parity gate
Makefile
Java emit parity lane now runs cmd_emit_java_testng test alongside the existing cmd_emit_java_junit test.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A link is cut, the path runs clean,
TestNG joins the parity machine,
Framework whispers fade to go
Provekit's simpler now, you know! 🐰✂️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Tighten parity gate and retire CLI language leaks' accurately summarizes the PR's main changes: extending the cross-language parity gate tests and removing language-specific logic from the CLI.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/testng-emitter-parity-gate

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@TSavo TSavo changed the title Include Java TestNG emitter in parity gate Tighten emit parity gate and keep CLI fallback language-blind May 29, 2026
@TSavo TSavo changed the title Tighten emit parity gate and keep CLI fallback language-blind Tighten parity gate and retire CLI language leaks May 29, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
implementations/rust/provekit-cli/src/cmd_emit.rs (1)

600-605: ⚡ Quick win

Consider adding test coverage for the extension parameter.

The current test verifies that the go+testing special-casing is removed when no extension is provided in the JSON. Consider also testing that when an extension field is present in the result JSON, it is correctly used in the output path.

✅ Suggested additional test case
 #[test]
 fn default_artifact_path_is_not_language_framework_special_cased() {
     let path = default_artifact_path("go", "testing", &json!({}));
 
     assert_eq!(path, PathBuf::from("provekit_emitted.go"));
 }
+
+#[test]
+fn default_artifact_path_uses_extension_from_result() {
+    let path = default_artifact_path("go", "testing", &json!({"extension": "test.go"}));
+
+    assert_eq!(path, PathBuf::from("provekit_emitted.test.go"));
+}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@implementations/rust/provekit-cli/src/cmd_emit.rs` around lines 600 - 605,
Add a test that verifies default_artifact_path uses an explicit "extension" from
the result JSON: create a JSON value with an "extension" field (e.g., ".mod" or
".custom"), call default_artifact_path("go","testing",&json!(...)) and assert
the returned PathBuf ends with the provided extension (e.g.,
"provekit_emitted.mod"); locate the test near default_artifact_path tests (e.g.,
alongside default_artifact_path_is_not_language_framework_special_cased) and
name it something like default_artifact_path_respects_extension to make intent
clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@implementations/rust/provekit-cli/src/cmd_emit.rs`:
- Around line 600-605: Add a test that verifies default_artifact_path uses an
explicit "extension" from the result JSON: create a JSON value with an
"extension" field (e.g., ".mod" or ".custom"), call
default_artifact_path("go","testing",&json!(...)) and assert the returned
PathBuf ends with the provided extension (e.g., "provekit_emitted.mod"); locate
the test near default_artifact_path tests (e.g., alongside
default_artifact_path_is_not_language_framework_special_cased) and name it
something like default_artifact_path_respects_extension to make intent clear.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9675ece1-cceb-4460-893e-ab696ebdf804

📥 Commits

Reviewing files that changed from the base of the PR and between 102a124 and b841f3d.

📒 Files selected for processing (5)
  • Makefile
  • implementations/rust/provekit-cli/src/cmd_emit.rs
  • implementations/rust/provekit-cli/src/cmd_link.rs
  • implementations/rust/provekit-cli/src/main.rs
  • implementations/rust/provekit-cli/tests/cli_surface.rs
💤 Files with no reviewable changes (2)
  • implementations/rust/provekit-cli/src/cmd_link.rs
  • implementations/rust/provekit-cli/src/main.rs

@TSavo TSavo merged commit 0500500 into main May 29, 2026
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant