terrymanu commented on PR #38852:
URL: https://github.com/apache/shardingsphere/pull/38852#issuecomment-4713935428
### Summary
- **Merge Decision: Mergeable**
- **Reason:** The latest meaningful commit keeps the MCP E2E optimization
within test infrastructure, preserves HTTP/STDIO confidence with targeted smoke
coverage, and the reviewed hardening paths pass local verification.
### Evidence
- The packaged distribution change now uses transport-specific reusable
homes, so HTTP and STDIO config rewrites no longer share the same mutable
prepared directory.
- The Proxy workflow shared fixture opt-in is explicitly named
`useSharedReadOnlyRuntimeFixture()`, and the only current call sites are
read-only completion/storage discovery cases.
- Programmatic HTTP keeps the heavier database-backed shared fixture
separate from protocol-only tests and from transaction-heavy isolated tests.
- No production MCP runtime behavior, user-facing configuration, dependency,
SQL parser behavior, or release artifact behavior is changed by the reviewed
commit.
- Local verification passed:
- `./mvnw -pl test/e2e/mcp -am -DskipTests test-compile -Pcheck` exit 0
- `./mvnw -pl test/e2e/mcp -Dtest=PackagedDistributionE2ETest
-Dsurefire.failIfNoSpecifiedTests=false -De2e.run.type=DOCKER test -Pcheck`
exit 0; 5 tests, 0 failures, 0 errors, 2 skipped
- `./mvnw -pl test/e2e/mcp
-Dtest=HttpProductionProxyEncryptWorkflowE2ETest,HttpProductionProxyMaskWorkflowE2ETest,HttpProductionProxyFeatureWorkflowContractE2ETest
-Dsurefire.failIfNoSpecifiedTests=false -De2e.run.type=DOCKER test -Pcheck`
exit 0; 24 tests, 0 failures, 0 errors
- `./mvnw spotless:apply -Pcheck -T1C` exit 0
- `./mvnw checkstyle:check -Pcheck -T1C` exit 0
### Review Details
- **Reviewed Scope:** Focused on commit
`5f4c19c769ba2314a8f580809538810beaa2683a` (`Harden shared fixture boundaries`)
and checked latest merge commit `52236563cfaf81f138a28df10bf560545afe06b3` for
conflict-marker or unique merge-resolution issues. Local base was
`5a9dd0de8f5b78a1195d4b30edc1f208e8bbb399`. GitHub files API cross-check
returned HTTP 504 during review, so I did not make unrelated-change findings
from the full PR file list.
- **Not Reviewed Scope:** Earlier PR commits from the previous unmerged
round, GitHub Actions / CI status, and unrelated storage-unit feature changes.
- **Verification:** Local commands listed above all passed; logs were
retained locally and paths are omitted from this public review summary.
- **Release Note / User Docs:** Not required. The reviewed change is test
infrastructure and maintainer workflow hardening only; it does not change
released user behavior or public MCP APIs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
