CHORE: Harden native metadata regression coverage

Require successful scalar NULL handling and exact describe counts. Add production-header cache, failure, concurrency, allocation and lifetime tests with active Release assertions, plus Windows/Linux/macOS CTest CI and test guidance.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: jahnvi480

.github/prompts/run-tests.prompt.md

@@ -122,6 +122,27 @@ Help the developer run tests to validate their changes. Follow this process base
 
 ## STEP 1: Choose What to Test
 
+### Native metadata invariants (no database)
+
+The standalone CMake tests in `tests/native` exercise the production metadata
+cache and child-handle invalidation helper without importing the Python package
+or connecting to SQL Server. They require a C++17 compiler, CMake, and ODBC
+headers (Windows SDK, `unixodbc-dev` on Linux, or `unixodbc` on macOS).
+The Native Metadata Tests workflow runs them on Windows, Linux, and macOS.
+
+```bash
+cmake -S tests/native -B build/native-metadata -DCMAKE_BUILD_TYPE=Release
+cmake --build build/native-metadata --config Release --parallel 2
+ctest --test-dir build/native-metadata -C Release --output-on-failure
+```
+
+Assertions remain enabled in Release. Cases cover stale-generation rejection,
+held snapshots, failure/EOF guards, concurrent invalidation, child isolation,
+reserve failure before strong-reference acquisition, and last-owner destruction
+outside the child-list lock. These native checks supplement, not replace, the
+live transaction tests, which skip when the driver does not preserve cursors.
+Native-only tests do not require the Python-test prerequisites above.
+
 ### Test Categories
 
 | Category | Description | When to Use |

.github/workflows/native-metadata-tests.yml

@@ -0,0 +1,49 @@
+name: Native Metadata Tests
+
+on:
+  pull_request:
+    types: [opened, reopened, synchronize, ready_for_review]
+    paths:
+      - 'mssql_python/pybind/result_metadata.hpp'
+      - 'mssql_python/pybind/connection/connection.cpp'
+      - 'tests/native/**'
+      - '.github/workflows/native-metadata-tests.yml'
+  push:
+    branches: [main]
+    paths:
+      - 'mssql_python/pybind/result_metadata.hpp'
+      - 'mssql_python/pybind/connection/connection.cpp'
+      - 'tests/native/**'
+      - '.github/workflows/native-metadata-tests.yml'
+
+permissions:
+  contents: read
+
+jobs:
+  native-metadata:
+    name: Native metadata (${{ matrix.os }})
+    runs-on: ${{ matrix.os }}
+    timeout-minutes: 10
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-latest, windows-latest, macos-latest]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
+        with:
+          persist-credentials: false
+      - name: Install ODBC headers (Linux)
+        if: runner.os == 'Linux'
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y unixodbc-dev
+      - name: Install ODBC headers (macOS)
+        if: runner.os == 'macOS'
+        run: brew install unixodbc
+      - name: Configure
+        run: cmake -S tests/native -B build/native-metadata -DCMAKE_BUILD_TYPE=Release
+      - name: Build
+        run: cmake --build build/native-metadata --config Release --parallel 2
+      - name: Test
+        run: ctest --test-dir build/native-metadata -C Release --output-on-failure

tests/native/CMakeLists.txt

@@ -0,0 +1,31 @@
+cmake_minimum_required(VERSION 3.15)
+project(mssql_python_native_tests LANGUAGES CXX)
+
+enable_testing()
+find_package(Threads REQUIRED)
+
+add_executable(result_metadata_tests result_metadata_tests.cpp allocation_failure.cpp)
+target_compile_features(result_metadata_tests PRIVATE cxx_std_17)
+target_include_directories(result_metadata_tests PRIVATE ../../mssql_python/pybind)
+target_link_libraries(result_metadata_tests PRIVATE Threads::Threads)
+
+if(WIN32)
+    target_compile_definitions(result_metadata_tests PRIVATE WIN32_LEAN_AND_MEAN NOMINMAX)
+else()
+    find_path(ODBC_INCLUDE_DIR sql.h PATHS /opt/homebrew/include /usr/local/include)
+    if(NOT ODBC_INCLUDE_DIR)
+        message(FATAL_ERROR "ODBC headers are required: install unixodbc-dev or unixodbc.")
+    endif()
+    target_include_directories(result_metadata_tests PRIVATE "${ODBC_INCLUDE_DIR}")
+endif()
+
+if(MSVC)
+    target_compile_options(result_metadata_tests PRIVATE /W4 /WX /UNDEBUG)
+else()
+    target_compile_options(result_metadata_tests PRIVATE -Wall -Wextra -Werror -UNDEBUG)
+endif()
+
+foreach(case_name IN ITEMS snapshots failures concurrent children allocation last_owner)
+    add_test(NAME result_metadata_${case_name} COMMAND result_metadata_tests ${case_name})
+    set_tests_properties(result_metadata_${case_name} PROPERTIES TIMEOUT 20)
+endforeach()

tests/native/allocation_failure.cpp

@@ -0,0 +1,32 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+#include <cstdlib>
+#include <memory>
+#include <new>
+
+struct TestHandle;
+extern std::weak_ptr<TestHandle> observedHandle;
+extern bool failAllocation;
+extern long ownersAtFailure;
+
+// Keep replacement allocation functions opaque to optimized test call sites.
+void* operator new(std::size_t size) {
+    if (failAllocation) {
+        failAllocation = false;
+        ownersAtFailure = observedHandle.use_count();
+        throw std::bad_alloc();
+    }
+    if (void* memory = std::malloc(size ? size : 1)) {
+        return memory;
+    }
+    throw std::bad_alloc();
+}
+
+void operator delete(void* memory) noexcept {
+    std::free(memory);
+}
+
+void operator delete(void* memory, std::size_t) noexcept {
+    std::free(memory);
+}

tests/native/result_metadata_tests.cpp

@@ -0,0 +1,208 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+#ifdef _WIN32
+#include <Windows.h>
+#endif
+#include "result_metadata.hpp"
+
+#include <cassert>
+#include <cstdio>
+#include <cstring>
+#include <new>
+#include <stdexcept>
+#include <thread>
+
+#ifdef NDEBUG
+#error Native metadata tests require assertions, including Release builds.
+#endif
+
+struct TestHandle {
+    ResultMetadataCache resultMetadata;
+    std::mutex* childMutex = nullptr;
+
+    ~TestHandle() {
+        if (childMutex) {
+            bool acquired = false;
+            std::thread observer([&] {
+                acquired = childMutex->try_lock();
+                if (acquired) {
+                    childMutex->unlock();
+                }
+            });
+            observer.join();
+            assert(acquired);
+        }
+    }
+};
+
+std::weak_ptr<TestHandle> observedHandle;
+bool failAllocation = false;
+long ownersAtFailure = -1;
+
+static std::shared_ptr<const ResultMetadata> MakeMetadata() {
+    auto metadata = std::make_shared<ResultMetadata>();
+    metadata->columns.push_back({u"owned", SQL_INTEGER, 10, 0, 1});
+    return metadata;
+}
+
+static void Populate(ResultMetadataCache& cache) {
+    const auto snapshot = cache.snapshot();
+    cache.publish(snapshot.generation, MakeMetadata());
+}
+
+static void TestSnapshots() {
+    ResultMetadataCache cache;
+    const auto initial = cache.snapshot();
+    assert(!initial.metadata);
+    auto metadata = MakeMetadata();
+    std::weak_ptr<const ResultMetadata> weak = metadata;
+    cache.publish(initial.generation, metadata);
+    auto held = cache.snapshot();
+    assert(held.metadata == metadata);
+    cache.clear();
+    assert(!cache.snapshot().metadata);
+    assert(cache.snapshot().generation != initial.generation);
+
+    Populate(cache);
+    const auto replacement = cache.snapshot();
+    cache.publish(initial.generation, metadata);
+    assert(cache.snapshot().metadata == replacement.metadata);
+    assert(held.metadata->columns.at(0).name == u"owned");
+    metadata.reset();
+    assert(!weak.expired());
+    held.metadata.reset();
+    assert(weak.expired());
+}
+
+static void TestFailures() {
+    ResultMetadataCache cache;
+    const SQLRETURN results[] = {SQL_SUCCESS, SQL_SUCCESS_WITH_INFO, SQL_NO_DATA,
+                                 SQL_ERROR, SQL_INVALID_HANDLE};
+    for (SQLRETURN result : results) {
+        Populate(cache);
+        const auto before = cache.snapshot();
+        {
+            ResultMetadataFailureGuard guard(cache, result);
+        }
+        const auto after = cache.snapshot();
+        if (SQL_SUCCEEDED(result) || result == SQL_NO_DATA) {
+            assert(after.metadata == before.metadata);
+            assert(after.generation == before.generation);
+        } else {
+            assert(!after.metadata);
+            assert(after.generation != before.generation);
+        }
+    }
+    Populate(cache);
+    SQLRETURN result = SQL_SUCCESS;
+    try {
+        ResultMetadataFailureGuard guard(cache, result);
+        throw std::runtime_error("conversion failure");
+    } catch (const std::runtime_error&) {
+        assert(!cache.snapshot().metadata);
+    }
+}
+
+static void TestConcurrentInvalidation() {
+    ResultMetadataCache cache;
+    const auto metadata = MakeMetadata();
+    std::thread invalidator([&] {
+        for (int i = 0; i < 1000; ++i) {
+            cache.clear();
+        }
+    });
+    for (int i = 0; i < 1000; ++i) {
+        const auto snapshot = cache.snapshot();
+        cache.publish(snapshot.generation, metadata);
+        if (snapshot.metadata) {
+            assert(snapshot.metadata->columns.at(0).name == u"owned");
+        }
+    }
+    invalidator.join();
+    cache.clear();
+    assert(!cache.snapshot().metadata);
+}
+
+static void TestChildren() {
+    std::mutex childMutex;
+    auto first = std::make_shared<TestHandle>();
+    auto second = std::make_shared<TestHandle>();
+    auto unrelated = std::make_shared<TestHandle>();
+    std::vector<std::weak_ptr<TestHandle>> children{first, {}, second};
+    Populate(first->resultMetadata);
+    Populate(second->resultMetadata);
+    Populate(unrelated->resultMetadata);
+    const auto held = first->resultMetadata.snapshot();
+    ClearChildResultMetadata(childMutex, children);
+    assert(!first->resultMetadata.snapshot().metadata);
+    assert(!second->resultMetadata.snapshot().metadata);
+    assert(unrelated->resultMetadata.snapshot().metadata);
+    assert(held.metadata->columns.at(0).name == u"owned");
+    ClearChildResultMetadata(childMutex, children);
+}
+
+static void TestAllocationFailure() {
+    std::mutex childMutex;
+    auto owner = std::make_shared<TestHandle>();
+    observedHandle = owner;
+    std::vector<std::weak_ptr<TestHandle>> children{owner};
+    Populate(owner->resultMetadata);
+    const auto before = owner->resultMetadata.snapshot();
+    failAllocation = true;
+    try {
+        ClearChildResultMetadata(childMutex, children);
+        assert(false);
+    } catch (const std::bad_alloc&) {
+        assert(ownersAtFailure == 1);
+        assert(owner->resultMetadata.snapshot().metadata == before.metadata);
+        assert(childMutex.try_lock());
+        childMutex.unlock();
+    }
+    assert(!failAllocation);
+    ClearChildResultMetadata(childMutex, children);
+    assert(!owner->resultMetadata.snapshot().metadata);
+}
+
+static void TestLastOwner() {
+    std::mutex childMutex;
+    auto owner = std::make_shared<TestHandle>();
+    owner->childMutex = &childMutex;
+    const std::weak_ptr<TestHandle> weak = owner;
+    std::vector<std::weak_ptr<TestHandle>> children{owner};
+    // Drop the external owner during invalidation, leaving only the helper's snapshot.
+    auto metadata = std::shared_ptr<ResultMetadata>(new ResultMetadata, [&](auto* value) {
+        owner.reset();
+        delete value;
+    });
+    const auto generation = owner->resultMetadata.snapshot().generation;
+    owner->resultMetadata.publish(generation, std::move(metadata));
+    ClearChildResultMetadata(childMutex, children);
+    assert(!owner && weak.expired());
+}
+
+int main(int argc, char** argv) {
+    if (argc != 2) {
+        std::fputs("Expected one native metadata test case\n", stderr);
+        return 2;
+    }
+    const char* name = argv[1];
+    if (std::strcmp(name, "snapshots") == 0) {
+        TestSnapshots();
+    } else if (std::strcmp(name, "failures") == 0) {
+        TestFailures();
+    } else if (std::strcmp(name, "concurrent") == 0) {
+        TestConcurrentInvalidation();
+    } else if (std::strcmp(name, "children") == 0) {
+        TestChildren();
+    } else if (std::strcmp(name, "allocation") == 0) {
+        TestAllocationFailure();
+    } else if (std::strcmp(name, "last_owner") == 0) {
+        TestLastOwner();
+    } else {
+        std::fprintf(stderr, "Unknown native metadata test case: %s\n", name);
+        return 2;
+    }
+    std::printf("%s passed\n", name);
+    return 0;
+}

tests/test_040_fetch_native_metadata.py

@@ -485,7 +485,7 @@ def test_result_metadata_transaction_preserved_cursor(metadata_cursor, operation
         else mssql_python.SQL_CURSOR_COMMIT_BEHAVIOR
     )
     if connection.getinfo(info) != 2:  # SQL_CB_PRESERVE
-        pytest.skip("Driver does not preserve cursors; native helper coverage is required")
+        pytest.skip("Driver does not preserve cursors; cache/helper coverage is in tests/native")
     cursor.execute(_query(["id"], 3))
     _assert_rows([cursor.fetchone()], [(1,)])
     if operation == "autocommit":
@@ -734,15 +734,9 @@ def test_result_metadata_all_null_rows(tmp_path, method):
                 scalar_null = []
                 scalar_status = native.DDBCSQLFetchOne(cursor.hstmt, scalar_null)
                 diagnostics = native.DDBCSQLGetAllDiagRecords(cursor.hstmt)
+                assert scalar_status == 0, (scalar_status, diagnostics)
                 assert scalar_null == [None]
-                if scalar_status == 0:
-                    assert diagnostics == []
-                    recovery_descriptions = 0
-                else:
-                    assert scalar_status == -1
-                    assert len(diagnostics) == 1 and "22002" in diagnostics[0][0], diagnostics
-                    assert "Indicator variable required but not supplied" in diagnostics[0][1]
-                    recovery_descriptions = 4 if {method!r} == "many" else 2
+                assert diagnostics == []
                 values = ",".join(f"({{i}})" for i in range(1, 16))
                 cursor.execute(
                     "SELECT CASE WHEN id%7=0 THEN NULL ELSE id END AS c0,"
@@ -778,9 +772,7 @@ def test_result_metadata_all_null_rows(tmp_path, method):
                 ]
                 if profiling:
                     stats = native.profiling.get_stats()
-                    # Only drivers/builds reporting a real scalar NULL error
-                    # require the additional post-error cache repopulations.
-                    expected_describes = (16 if {method!r} == "one" else 17) + recovery_descriptions
+                    expected_describes = 16 if {method!r} == "one" else 17
                     assert stats["ddbc::SQLDescribeCol::driver_call"]["calls"] == expected_describes, stats
                     assert stats["ddbc::sql_variant::null_probe"]["calls"] == 15
                     assert stats["ddbc::sql_variant::subtype"]["calls"] == 13