Get E2e Tests working in Windows - #149
Open
Theekshna wants to merge 4 commits into
Open
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
Comment on lines
+29
to
+32
| /// Reserved for future field-level descriptor access; unused while the | ||
| /// Driver Manager only requires the handles to exist. | ||
| #[allow(dead_code)] | ||
| pub(crate) inner: Mutex<DescState>, |
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_env_attr.rsmssql-odbc/src/api/get_functions.rsmssql-odbc/src/api/get_info.rsmssql-odbc/src/api/get_stmt_attr.rsmssql-odbc/src/handles/desc.rs🔗 Quick Links |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| ODBCTest::SetUp(); | ||
| if (!ODBCTestConfig::Instance().HasConnection()) { | ||
| GTEST_SKIP() << "No connection configured – set ODBC_TEST_SERVER or ODBC_TEST_CONNSTR"; | ||
| FAIL() << "No connection configured – set ODBC_TEST_SERVER or ODBC_TEST_CONNSTR"; |
Contributor
There was a problem hiding this comment.
Can we update the readme.md file https://github.com/microsoft/mssql-rs/pull/149/changes#:~:text=Tests%20that%20require,to%20enable%20them%3A , which still says connected tests are skipped when no connection is configured.
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