test: support captured runtime values in SQL cases - #202
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Ariznawlll
left a comment
Ariznawlll
left a comment
There was a problem hiding this comment.
Requesting changes for one fail-closed issue in the new capture control path. The parser tests and the full unit suite pass on JDK 8, but there is no execution-level test covering this failure boundary.
Ariznawlll
left a comment
Ariznawlll
left a comment
There was a problem hiding this comment.
The original fail-closed behavior is now implemented, but this head is not merge-ready: the new execution test does not compile on the repository-required JDK 8, and the genRS capture-failure early return bypasses statement/result-set and test-database cleanup.
Ariznawlll
left a comment
Ariznawlll
left a comment
There was a problem hiding this comment.
Re-reviewed head 74732b9. The previous blockers are addressed: capture failures now fail closed in both run and genRS; genRS routes failures through statement/result-set and test-database cleanup; the JDK 8 character literal is valid; and execution tests cover query-error, empty-result, and NULL-result cleanup. I also ran the full Maven unit suite locally with Java compilation targeting 1.8: 74 tests passed. Non-blocking follow-up: the current GitHub CI jobs execute the checked-in prebuilt mo-tester JAR and do not compile or run tests from src/, so adding an explicit Maven build/test job would make future Java-source PR checks meaningful.
2 participants
Adds @capture:name for scalar runtime values used by later {{name}} placeholders. This enables a deterministic PITR restore test without unsupported prepared RESTORE statements.\n\nValidation: mvn -Dtest=ScriptParserTest test; local PITR restore case 17/17.