[Copilot]

TSqlParser.java had significant uncovered control flow across facade overloads, builder/getter accessors, parse exception handling, and main(String[] args) argument paths. This update adds focused unit coverage for those paths and introduces a minimal test seam to reliably exercise the parser-exception branch.

• Facade overloads + builder/getters

  • Added tests for:
    • parse(Reader)
    • parse(Reader, SqlVersion)
    • fluent builder behavior (initialQuotedIdentifiers, sqlVersion, build)
    • getter values (isInitialQuotedIdentifiersOn, getTargetVersion)
  • Included explicit assertion for sqlVersion(null) fallback to SQL180.

• Deterministic exception-branch coverage

  • Added package-private factory method in TSqlParser:
    • TSql180Parser createUnderlyingParser()
  • parseScript(Reader) now uses the factory, allowing a test override to force an internal parser exception and validate ParseError wrapping behavior.

• CLI entrypoint coverage (main)

  • Added subprocess-based tests for:
    • help output (-h)
    • raw SQL success path
    • invalid SQL failure path (non-zero exit)
    • file input via -f <path>
    • missing file error path (non-zero exit)

TSqlParser parser = new TSqlParser() {
    @Override
    TSql180Parser createUnderlyingParser() {
        return new TSql180Parser() {
            @Override
            public TSqlFragment parse(java.io.Reader reader, List errors) {
                throw new RuntimeException("boom");
            }
        };
    }
};

• Fixes Add unit test coverage for TSqlParser facade overloads, builder, and CLI #28

[Copilot]

🟡 Changes recommended

The subprocess-based main tests won't register JaCoCo coverage and the tests reference an undefined requirement (REQ-028), undermining the stated coverage/traceability goals.

2 open findings

• CLI tests spawn uninstrumented JVM, missing main coverage · New
• Tests reference nonexistent REQ-028 in traceability comments · New

What changed in this PR

This PR expands unit-test coverage for TSqlParser, the public facade/CLI entry point of the core T-SQL parsing library. The linked issue (#28) reports that TSqlParser.java line coverage was only 33.7%, with the Reader overloads, fluent Builder, getters, the parse-exception branch, and the main CLI path all uncovered. To make the internal exception branch deterministically testable, the PR also introduces a small package-private seam (createUnderlyingParser()) in production code.

Changes:

• Added tests for the Reader overloads, fluent Builder (including sqlVersion(null) fallback to SQL180), and getters.
• Introduced a package-private createUnderlyingParser() factory in TSqlParser and used it in parseScript(Reader), enabling a test override that forces the parser-exception/ParseError wrapping branch.
• Added subprocess-based tests for main (-h, raw SQL success, invalid SQL failure, -f <file>, and missing-file error paths).

File	Description
tsql-scriptdom-core/​src/​main/​java/​io/​github/​fupacat/​TSqlParser.java	Adds package-private createUnderlyingParser() factory and uses it in parseScript(Reader) as a test seam; behavior-preserving.
tsql-scriptdom-core/​src/​test/​java/​io/​github/​fupacat/​TSqlParserFacadeTest.java	Adds facade/builder/getter/exception tests plus subprocess-based main CLI tests and a runMain helper.

Notes for the author:

• The subprocess-launched JVM in runMain is not instrumented by the JaCoCo agent, so the main method lines will likely remain uncovered despite these tests validating behavior — the coverage objective for main from issue #28 may not be met this way.
• The new TRACEABILITY_MAPPING: REQ-028 comments reference a requirement that is not defined in context/SRS.md (requirements only go up to REQ-023), breaking the traceability chain used by the project.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The main tests run in an un-instrumented child JVM so they don't raise JaCoCo coverage for main() (the PR's stated goal), and the traceability mappings reference an undefined REQ-028.

2 open findings

• CLI tests spawn uninstrumented JVM, missing main coverage
• Tests reference nonexistent REQ-028 in traceability comments

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

All new tests use the traceability mapping REQ-028, which is not defined in context/SRS.md (only REQ-001..REQ-023 exist), violating the repository's established MCSWE v3 traceability convention.

2 open findings

• CLI tests spawn uninstrumented JVM, missing main coverage
• Tests reference nonexistent REQ-028 in traceability comments

🧠 Review effort: Balanced