Get E2e Tests working in Windows - #149
Merged
Merged
Conversation
SQL_DIAG_CLASS_ORIGIN and SQL_DIAG_SUBCLASS_ORIGIN are synthesized by the Driver Manager, not the driver. unixODBC returns ODBC 3.0 for the 08001 connect error while the Windows DM returns ISO 9075, so the two tests only passed when run against the Microsoft driver. The driver's own origin logic is already covered by get_diag.rs unit tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Windows Driver Manager compatibility for mssql-odbc and expands Windows E2E/parity testing.
Changes:
- Adds required ODBC connection, metadata, environment, statement, and descriptor support.
- Introduces implicit statement descriptors and diagnostic fields.
- Adds Unicode Windows builds and msodbcsql parity runs.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
mssql-odbc/tests/e2e/tests/smoke_test.cpp |
Fails when live configuration is missing. |
mssql-odbc/tests/e2e/tests/num_result_cols_test.cpp |
Requires live configuration. |
mssql-odbc/tests/e2e/tests/more_results_test.cpp |
Requires live configuration. |
mssql-odbc/tests/e2e/tests/execute_test.cpp |
Requires live configuration. |
mssql-odbc/tests/e2e/tests/exec_direct_test.cpp |
Requires live configuration. |
mssql-odbc/tests/e2e/tests/driver_connect_test.cpp |
Improves diagnostic-state lookup. |
mssql-odbc/tests/e2e/tests/describe_col_test.cpp |
Requires live configuration. |
mssql-odbc/tests/e2e/tests/alloc_stmt_test.cpp |
Requires live configuration. |
mssql-odbc/tests/e2e/tests/alloc_env_test.cpp |
Requires live configuration. |
mssql-odbc/tests/e2e/run_e2e.sh |
Adds configurable Unicode mode. |
mssql-odbc/tests/e2e/run_e2e.ps1 |
Adds Windows parity execution and reporting. |
mssql-odbc/tests/e2e/README.md |
Documents Windows parity and Unicode builds. |
mssql-odbc/tests/e2e/CMakeLists.txt |
Configures platform-specific Unicode compilation. |
mssql-odbc/src/handles/stmt.rs |
Owns four implicit descriptors. |
mssql-odbc/src/handles/mod.rs |
Registers the descriptor module. |
mssql-odbc/src/handles/desc.rs |
Defines implicit descriptor handles. |
mssql-odbc/src/api/set_connect_attr.rs |
Accepts SQL_ATTR_ANSI_APP. |
mssql-odbc/src/api/odbc_types.rs |
Adds ODBC constants and identifiers. |
mssql-odbc/src/api/mod.rs |
Registers new API modules. |
mssql-odbc/src/api/get_stmt_attr.rs |
Returns implicit descriptor handles. |
mssql-odbc/src/api/get_info.rs |
Implements driver capability metadata. |
mssql-odbc/src/api/get_functions.rs |
Reports implemented ODBC functions. |
mssql-odbc/src/api/get_env_attr.rs |
Retrieves ODBC environment version. |
mssql-odbc/src/api/get_diag.rs |
Expands diagnostic-field support. |
mssql-odbc/src/api/exports.rs |
Exports the new ODBC entry points. |
mssql-odbc/src/api/driver_connect.rs |
Exposes the shared connection core. |
mssql-odbc/src/api/connect.rs |
Adds SQLConnectW. |
Comments suppressed due to low confidence (2)
mssql-odbc/src/api/exports.rs:642
- This exported FFI entry point bypasses
crate::ffi_entry!, so a panic can unwind across the C ABI boundary. Delegate to a separately wrapped implementation instead of implementing the stub directly inexports.rs.
super::odbc_types::SQL_ERROR
mssql-odbc/src/api/exports.rs:673
- This exported descriptor entry point also lacks the mandatory
crate::ffi_entry!panic boundary. Put the validation/error logic in a wrapped implementation and keep this export as a tracing-initializing delegate, otherwise a panic may unwind into the Driver Manager.
super::odbc_types::SQL_ERROR
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql-odbc/src/api/connect.rsmssql-odbc/src/api/exports.rsmssql-odbc/src/api/get_diag.rsmssql-odbc/src/api/get_env_attr.rsmssql-odbc/src/api/get_functions.rsmssql-odbc/src/api/get_info.rsmssql-odbc/src/handles/desc.rsmssql-tds/src/connection/tds_client.rs🔗 Quick Links |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve conflicts from main's unified execute + SQLRowCount work: - exports.rs / mod.rs: keep both new module sets; single SQLGetStmtAttrW - stmt.rs: keep desc-handle and odbc_types imports from both sides - set_stmt_attr.rs: fold the four implicit-descriptor arms into main's combined SQLGetStmtAttr, drop the redundant get_stmt_attr module - odbc_types.rs: remove duplicate descriptor attribute constants Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3b4d2d23-4805-4c06-97fd-0ea460a812ca
- Dispatch SQLGetDiagRec/FieldW for SQL_HANDLE_DESC so the implicit descriptor handles returned by SQLGetStmtAttrW report diagnostics instead of SQL_INVALID_HANDLE. - Report SQL_DBMS_VER from the version negotiated in the LOGINACK token via a new TdsClient::server_version() accessor, replacing the hardcoded value. - Correct the e2e README: connected tests now fail (not skip) when no connection is configured. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3b4d2d23-4805-4c06-97fd-0ea460a812ca
5 tasks
Saurabh Singh (saurabh500)
approved these changes
Aug 4, 2026
Saurabh Singh (saurabh500)
enabled auto-merge (squash)
August 4, 2026 11:42
Contributor
|
Another followup PR will be sent to report true Windows Code Coverage. |
Shiwani Gupta (shiwanigupta0809)
approved these changes
Aug 4, 2026
Saurabh Singh (saurabh500)
added a commit
that referenced
this pull request
Aug 4, 2026
…149 squash conflicts) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3b4d2d23-4805-4c06-97fd-0ea460a812ca
Vahid (Vahid-b)
added a commit
that referenced
this pull request
Aug 4, 2026
Two conflicts from #149: - exports.rs: main still had the SQLGetConnectAttrW stub returning SQL_ERROR; this branch implements it, so the implementation is kept. - set_connect_attr.rs: main added SQL_ATTR_ANSI_APP to the accept-and-ignore arm. That arm is kept with SQL_ATTR_ANSI_APP, minus SQL_ATTR_LOGIN_TIMEOUT, which this branch handles in its own arm above; leaving it in both would be an unreachable pattern. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Summary
Expands the phase-1
mssql-odbcdriver with the ODBC 3.x entry points the Windows Driver Manager requires during connection and statement setup, and aligns their behavior with the msodbcsql reference driver. All new handlers were verified against the msodbcsql source (headers +sqlcinfo.cpp/sqlcmisc.cpp) for constant values, bit-packing, and null-pointer semantics.Changes
SQLConnectW,SQLGetFunctions,SQLGetInfoW,SQLGetEnvAttr/SQLSetEnvAttr,SQLGetStmtAttrW,SQLGetConnectAttrW,SQLGetDiagFieldW, andSQLGetDescFieldW(with a newdescdescriptor handle type and the four implicit descriptors onStmtHandle).SQLGetFunctionsuses the reference's ODBC3 bit-packing (idx = id>>4,1 << (id & 0xF), 250-word map) and reports a correct strict-subset of implemented functions (adds the previously-missingSQLConnect/SQLRowCount);SQLGetFunctionsandSQLGetStmtAttrWnow treat a null out-pointer as a benignSQL_SUCCESSno-op instead of returning HY009/SQL_ERROR.odbc_types.rs, all audited againstsql.h/sqlext.h/sqlucode.h/sqlspi.h(fixedSQL_API_SQLCONNECTto7); minor diagnostics/named-constant cleanups inget_diag.rsandget_info.rs.get_diag_fieldcoverage,compareWith msodbcsqlsupport inrun_e2e.ps1, an optional UNICODE build toggle in CMake, and switched connection-less tests fromGTEST_SKIPto a hard failure so missing config surfaces instead of silently passing.Validation
Built clean and
343lib tests pass undercargo +ms-stable. Note: local verification used+ms-stablerather than the pinned1.95toolchain (msrustup constraint), and coverage/nextest were substituted withcargo test/clippy— CI should re-confirm on the pinned toolchain.Related Issues
closes AB#46564
Checklist
cargo bfmtpassescargo bclippypassescargo btestpasses