CHORE: Add scheduled mssql-odbc pytest pipeline

[gargsaumya]

Work Item / Issue Reference

AB#48169

---

Summary

Adds a dedicated scheduled Azure Pipeline that validates mssql-python with MSSQL_PYTHON_NATIVE_PROVIDER=mssql-odbc once daily against the latest GitHub main.

• Installs the stable mssql-python-rs==0.1.0 runtime and verifies the resolved mssqlodbc payload.
• Connects to SQL Server and executes SELECT 1 as a blocking provider preflight.
• Runs every standard pytest file in an isolated process and publishes merged JUnit results.
• Reports functional failures, crashes, timeouts, and aggregate-budget exhaustion as SucceededWithIssues; setup, provider-preflight, harness, and publication failures remain blocking.
• Does not enable or claim Conda support for mssql-odbc.

Validation

• Pipeline YAML parsed and daily main schedule assertions passed.
• Bash syntax and Python syntax/Black checks passed.
• Linux regression tests cover clean pass, budget exhaustion, crash/malformed-JUnit handling, authenticated advisory status, and blocking harness failures.
• Stable mssql-python-rs-wheels==0.1.0 presence was verified in the configured public feed.

Dependency

Uses the released mssql-python-rs==0.1.0 distribution from the stable mssql-python-rs-wheels==0.1.0 NuGet transport. An Azure DevOps administrator must register the pipeline definition after the YAML reaches main.

[Copilot]

🟡 Changes recommended

Unresolved moderate findings affect failure classification, timeout headroom, setup handling, and JUnit validity.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a scheduled daily Azure Pipeline to validate mssql-python with the mssql-odbc provider.

Changes:

• Adds provider and SQL connectivity preflight checks.
• Runs isolated pytest files with timeout and JUnit reporting.
• Adds Docker setup, scheduling, result publication, and cleanup.

File summaries

File	Summary
eng/scripts/verify_mssql_odbc_provider.py	Validates provider selection, payload, and connectivity; no final comments.
eng/scripts/run-mssql-odbc-tests.sh	Requires changes for setup failure handling, pytest exit-code classification, timeout budgeting, and malformed JUnit protection.
eng/pipelines/mssql-odbc-daily-validation-pipeline.yml	Requires changes for job timeout headroom and blocking container setup failures.

Review details

Suppressed comments (4)

eng/pipelines/mssql-odbc-daily-validation-pipeline.yml:101

• This step does not enable errexit, so if docker inspect $(sqlContainer) fails (for example, the SQL container has exited), the command substitution leaves an empty IP and the script still invokes the test container. A resulting docker exec exit 1 is then treated as an advisory compatibility failure, allowing an infrastructure failure to finish as SucceededWithIssues; use set -euo pipefail here so host/setup errors remain blocking (the docker exec is already guarded by || rc=$?).

      set -uo pipefail

eng/scripts/run-mssql-odbc-tests.sh:15

• Because this script intentionally omits set -e, a failure to create the results directory is ignored. Subsequent pytest/report operations can then produce a nonzero result that is classified as advisory, so a harness setup failure does not remain blocking as described. Check this setup operation explicitly and exit 2 on failure.

mkdir -p "$RESULTS_DIR"

eng/scripts/run-mssql-odbc-tests.sh:136

• A timeout/crash can leave a non-empty but truncated JUnit XML file while pytest is writing it. The size-only check then skips write_stub, so PublishTestResults receives malformed XML instead of the required process-level result. Validate that the existing report is well-formed (or replace it with a stub) before keeping it.

    if [ ! -s "$report" ]; then
        write_stub "$name" error "Pytest exited $rc without producing JUnit" "$report"
    fi

eng/scripts/run-mssql-odbc-tests.sh:106

• The per-file slice and remaining calculations do not include --kill-after=60s. When a test reaches its timeout, this command can run up to another minute after the slice (including when the final slice is only 31 seconds), so the advertised 110-minute total budget can be exceeded and the job can lose time reserved for result publication and cleanup. Account for the kill grace period when selecting slice, or otherwise bound the termination grace within the total budget.

    echo "##[group]$test_file"
    timeout --kill-after=60s "${slice}s" \
        python -m pytest "$test_file" -v --junitxml="$report" \

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

---

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

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 100%

🎯 Overall Coverage 83%

📈 Total Lines Covered: 8455 out of 10100 📁 Project: mssql-python

---

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

---

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.performance_counter.hpp: 0.7%
mssql_python.pybind.logger_bridge.cpp: 57.9%
mssql_python.pybind.ddbc_bindings.h: 64.1%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.row.py: 77.6%
mssql_python.pybind.ddbc_bindings.cpp: 77.7%
mssql_python.pybind.connection.connection_pool.cpp: 81.8%
mssql_python.logging.py: 86.2%
mssql_python.pooling.py: 90.1%
mssql_python.pybind.py_type_cache.hpp: 91.6%

---

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report

[Copilot]

🔵 Needs a closer look

Unresolved findings can misreport harness, skipped-test, crash/timeout, and result-generation failures.

Review details

Suppressed comments (6)

eng/pipelines/mssql-odbc-daily-validation-pipeline.yml:104

• This wrapper uses set -uo pipefail, so a failed docker inspect is not fatal. It leaves SQLSERVER_IP empty, still runs pytest with an invalid connection string, and the resulting exit 1 is then marked SucceededWithIssues; discovery of the SQL container/IP should remain a blocking harness failure.

      set -uo pipefail

eng/pipelines/mssql-odbc-daily-validation-pipeline.yml:118

• The raw docker exec status is passed directly into this case, but Docker can also return 1 for an execution/infrastructure error, not just for the runner's intentional pytest-failure status. A stopped container or failed exec can therefore be reported as an advisory compatibility failure instead of blocking; encode the runner's rc=1 separately (or verify a completion marker) before mapping it to SucceededWithIssues.

        ' || rc=$?

eng/scripts/run-mssql-odbc-tests.sh:140

• Checking only whether the XML file is nonempty loses crashes or timeouts that happen after pytest has written its JUnit file (for example, a native teardown crash). The script increments crashed/timed_out and returns advisory, but PublishTestResults then sees only the pre-crash results, so the process error is absent from the merged report. Add a separate error stub whenever a non-success, non-skipped process exit occurs, even if $report already exists.

    if [ ! -s "$report" ]; then
        write_stub "$name" error "Pytest exited $rc without producing JUnit" "$report"
    fi

eng/scripts/run-mssql-odbc-tests.sh:124

• This repository's pytest.ini adds -m "not stress", and test_011_singlethreaded_stress.py, test_020_multithreaded_stress.py, and test_021_concurrent_connection_perf.py contain only @pytest.mark.stress tests. Running those files individually therefore returns pytest exit code 5 on every healthy run; counting each as skipped makes the final skipped > 0 check return 1, so the pipeline is always marked SucceededWithIssues instead of passing when all standard (non-stress) tests pass. Exclude stress-only files from the per-file list or distinguish marker deselection from an unexpected empty test file without incrementing the advisory failure count.

        5)
            write_stub "$name" skipped "No tests collected" "$report"
            skipped=$((skipped + 1))

eng/scripts/run-mssql-odbc-tests.sh:140

• An empty/missing JUnit file is replaced with an error stub, but the counters are left unchanged. If pytest returns 0 while report creation fails (for example, the results directory is unwritable), passed remains incremented and the final status is 0, so this harness failure is incorrectly reported as a clean pass. Treat missing reports for non-crash/non-timeout exits as blocking.

    if [ ! -s "$report" ]; then
        write_stub "$name" error "Pytest exited $rc without producing JUnit" "$report"
    fi

eng/scripts/run-mssql-odbc-tests.sh:114

• pytest exits 0 when a file is collected but every test is skipped, including module-level skips. Counting every rc=0 file as passed means an all-skipped run can satisfy the check at lines 145-147 and be reported as a clean pass; use the generated JUnit counts (or another collected/runnable-test signal) before incrementing passed so skipped environments cannot masquerade as successful validation.

        0)
            passed=$((passed + 1))
            ;;

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Critical and moderate issues remain in dependency setup, failure handling, cleanup, and test-result classification.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

eng/pipelines/mssql-odbc-daily-validation-pipeline.yml:155

• The cleanup script does not enable errexit. If setup_sql_container.py --cleanup returns nonzero (for example, removal or ownership verification fails), docker rm -f ... || true becomes the final successful command and the always-run cleanup step reports success while the SQL container may remain. Propagate the helper's failure instead of masking it.

  - script: |
      python3 eng/scripts/setup_sql_container.py --cleanup \
        --name "$(sqlContainer)" --owner "$(Build.BuildId).$(System.JobId)"
      docker rm -f $(testContainer) || true

• Files reviewed: 3/3 changed files
• Comments generated: 4
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Address the three moderate findings affecting preflight reliability, timeout reporting, and complete test execution.

Review details

Suppressed comments (3)

eng/pipelines/mssql-odbc-daily-validation-pipeline.yml:117

• The configured 110-minute total budget can stop the loop before all discovered test_*.py files run; when the budget is nearly exhausted, the remaining files only receive skipped JUnit stubs. That contradicts the stated guarantee that the pipeline runs every standard pytest file. Please either size/remove this cutoff so the complete file list executes, or explicitly document and surface this as a partial validation result.

        -e PYTEST_FILE_TIMEOUT=10m \
        -e PYTEST_TOTAL_BUDGET=110m \

eng/scripts/run-mssql-odbc-tests.sh:86

• When the total budget is exhausted after at least one file has run, this branch only writes skipped stubs and exits the loop; failed, crashed, and timed_out remain zero, so the final status check returns 0 and the pipeline reports a pass even though files were not executed. Count budget exhaustion as a timeout (or otherwise return the advisory failure status) before breaking.

    if [ "$remaining" -le 30 ]; then

eng/scripts/verify_mssql_odbc_provider.py:22

• The existing mssql-odbc smoke tests document that the Rust driver can panic during interpreter teardown after a successful query, so they accept a success marker in stdout instead of the subprocess exit code. This script relies on normal Python exit status; that known post-query panic can therefore make the blocking provider preflight fail even when SELECT 1 succeeded. Run the preflight in a child process and validate a success marker (as tests/test_026_odbc_provider.py does), while still failing when the marker is absent.

    with mssql_python.connect(os.environ["DB_CONNECTION_STRING"]) as connection:
        with connection.cursor() as cursor:
            cursor.execute("SELECT 1")
            assert cursor.fetchone()[0] == 1

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The runner can falsely pass incomplete suites, while provider, infrastructure, and cleanup failures may be misclassified or hidden.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

eng/pipelines/mssql-odbc-daily-validation-pipeline.yml:98

• The existing mssql-odbc smoke tests document that the Rust driver can panic during teardown after a successful query and therefore validate a success marker instead of the subprocess exit code (tests/test_026_odbc_provider.py:396-403, 446-461). This preflight prints MSSQL_ODBC_PREFLIGHT_OK but runs as a normal set -e command, so that known teardown behavior can make docker exec fail and block the pipeline even after SELECT 1 succeeded. Capture and validate the marker while still failing when it is absent.

          python eng/scripts/verify_mssql_odbc_provider.py

eng/pipelines/mssql-odbc-daily-validation-pipeline.yml:161

• This script does not enable fail-fast behavior, and the final docker rm ... || true returns success even when setup_sql_container.py --cleanup fails. The cleanup step can therefore hide an SQL-container removal failure and report success, leaving leaked resources or masking an infrastructure problem. Preserve the cleanup command's status while still attempting to remove the test container.

  - script: |
      python3 eng/scripts/setup_sql_container.py --cleanup \
        --name "$(sqlContainer)" --owner "$(Build.BuildId).$(System.JobId)"
      docker rm -f $(testContainer) || true

eng/scripts/verify_mssql_odbc_provider.py:22

• This preflight runs as a direct Python process, but the Rust provider is known to sometimes panic during interpreter teardown after a successful query; the existing provider smoke test therefore treats a success marker in stdout as authoritative instead of requiring a zero subprocess exit (tests/test_026_odbc_provider.py:401-403, 457-461). Because the pipeline invokes this script under set -e, a teardown-only nonzero exit will fail the blocking preflight even after MSSQL_ODBC_PREFLIGHT_OK was printed. Run the query in a child and validate a flushed success marker, or otherwise preserve the repository's marker-based teardown handling.

    with mssql_python.connect(os.environ["DB_CONNECTION_STRING"]) as connection:
        with connection.cursor() as cursor:
            cursor.execute("SELECT 1")
            assert cursor.fetchone()[0] == 1

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Address the identified preflight, test-runner failure classification, timeout, and JUnit publication issues.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

eng/pipelines/mssql-odbc-daily-validation-pipeline.yml:148

• continueOnError: true suppresses failures of the JUnit publisher itself; failTaskOnFailedTests: false already keeps ordinary failed test cases advisory. A malformed report or publishing-service failure can therefore leave the daily validation non-blocking without the promised merged results. Remove this override so result-publication/infrastructure failures remain visible as blocking failures.

    continueOnError: true

eng/pipelines/mssql-odbc-daily-validation-pipeline.yml:124

• The docker exec exit status is copied directly into rc, but a Docker/container/command-start failure can also return 1. The case below treats every 1 as an advisory pytest assertion failure and marks the task SucceededWithIssues, so an infrastructure or harness failure can be hidden; distinguish the runner's rc=1 from an exec failure before applying the advisory mapping.

        $(testContainer) bash -c '
          source /opt/venv/bin/activate
          chmod +x eng/scripts/run-mssql-odbc-tests.sh
          eng/scripts/run-mssql-odbc-tests.sh
        ' || rc=$?

eng/scripts/run-mssql-odbc-tests.sh:94

• When the total budget is nearly exhausted, every remaining file is emitted as a skipped stub and timed_out is left unchanged. The final check therefore returns 0, so the pipeline can report a complete pass even though those files never ran; budget exhaustion should be classified as a timeout/advisory failure rather than an intentional skip.

            write_stub "$rest_name" skipped "Total test budget of $TOTAL_BUDGET exhausted" \
                "$RESULTS_DIR/results-$rest_name.xml"
            skipped=$((skipped + 1))

eng/scripts/verify_mssql_odbc_provider.py:27

• The Rust provider's existing smoke tests document that teardown can panic after a successful query and therefore validate a success marker instead of the child exit code. This preflight is invoked directly, so a teardown-only nonzero exit still fails the blocking step even when MSSQL_ODBC_PREFLIGHT_OK was printed. Run the query behind a wrapper/child and require the marker while tolerating only the known teardown exit condition.

    loaded_info = mssql_python.get_native_provider_info()
    assert loaded_info["id"] == "mssql-odbc", loaded_info
    assert loaded_info["frozen"] is True, loaded_info
    print("MSSQL_ODBC_PREFLIGHT_OK", loaded_info)

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The provider preflight script relies on Python assert (which can be optimized away) and the runner’s process-stub JUnit message can be misleading when no JUnit was actually produced.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

eng/scripts/verify_mssql_odbc_provider.py:15

• verify_mssql_odbc_provider.py uses assert for runtime validation of the provider selection and driver path. Running Python with optimizations (-O / PYTHONOPTIMIZE) strips asserts, which would let the preflight silently pass even if the provider/driver is wrong. Prefer explicit checks that raise an exception with the same diagnostic payload.
  eng/scripts/verify_mssql_odbc_provider.py:27
• The preflight query and post-load validation also rely on assert (e.g., assert cursor.fetchone()[0] == 1, assert loaded_info["frozen"] is True). If asserts are disabled, the script could print the success marker even when the query/provider-freeze checks fail. Use explicit checks and raise a clear exception instead.

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[github-actions]

PR Performance Report

Performance assessment pending.

Waiting for the matching performance run for head 705d3003b833da1c810efa564da86935dae71246.

[Copilot]

🟡 Changes recommended

The new contract tests and preflight script have concrete robustness gaps that can cause cross-platform CI failures and/or obscure diagnostics in common failure modes.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 4
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The pipeline, runner, and preflight changes are internally consistent, match existing provider-selection contracts, and include targeted contract tests to prevent regressions.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The changes are cohesive and validated with tests/pipeline contracts, and the remaining feedback is limited to maintainability polish rather than functional correctness.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite