This is an automated email from the ASF dual-hosted git repository.
CurtHagenlocher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 976184f8c fix(csharp): address flaky driver manager tests (#4324)
976184f8c is described below
commit 976184f8c3011f81d8821a2a4983e942586354ad
Author: davidhcoe <[email protected]>
AuthorDate: Tue May 19 11:35:04 2026 -0400
fix(csharp): address flaky driver manager tests (#4324)
- Addresses the race condition in the driver manager tests that are
causing issues with check-ins
Co-authored-by: David Coe <>
---
.../DriverManager/DriverManagerSecurity.cs | 22 ++++++++++
.../DriverManagerSecurityCollection.cs | 50 ++++++++++++++++++++++
.../DriverManagerSecurityIntegrationTests.cs | 2 +-
.../DriverManager/DriverManagerSecurityTests.cs | 12 ++++++
.../DriverManager/ManagedDriverLoaderTests.cs | 12 ++++++
5 files changed, 97 insertions(+), 1 deletion(-)
diff --git
a/csharp/src/Apache.Arrow.Adbc/DriverManager/DriverManagerSecurity.cs
b/csharp/src/Apache.Arrow.Adbc/DriverManager/DriverManagerSecurity.cs
index fc41996a3..9b4d81aa6 100644
--- a/csharp/src/Apache.Arrow.Adbc/DriverManager/DriverManagerSecurity.cs
+++ b/csharp/src/Apache.Arrow.Adbc/DriverManager/DriverManagerSecurity.cs
@@ -37,7 +37,19 @@ namespace Apache.Arrow.Adbc.DriverManager
/// When set, all driver load attempts will be logged.
/// </summary>
/// <remarks>
+ /// <para>
/// This property is thread-safe. Set to <c>null</c> to disable audit
logging.
+ /// </para>
+ /// <para>
+ /// <b>Testing note:</b> this is process-wide mutable state. Test
classes
+ /// that assign this property -- or that exercise the
+ /// <see cref="AdbcDriverManager"/> load path, which reads it on every
+ /// load -- must join the same xUnit collection
+ /// (<c>DriverManagerSecurityCollection.Name</c>) so they are
serialized
+ /// against each other. Otherwise concurrent tests can null out the
+ /// logger between "install" and "load", producing flaky
+ /// <c>Assert.Single</c> failures on captured-attempt lists.
+ /// </para>
/// </remarks>
public static IDriverLoadAuditLogger? AuditLogger
{
@@ -71,6 +83,16 @@ namespace Apache.Arrow.Adbc.DriverManager
/// allowlist to restrict which drivers can be loaded, preventing
/// potential arbitrary code execution attacks.
/// </para>
+ /// <para>
+ /// <b>Testing note:</b> this is process-wide mutable state. Test
classes
+ /// that assign this property -- or that exercise the
+ /// <see cref="AdbcDriverManager"/> load path, which reads it on every
+ /// load -- must join the same xUnit collection
+ /// (<c>DriverManagerSecurityCollection.Name</c>) so they are
serialized
+ /// against each other. Otherwise a concurrent test can install a
+ /// restrictive allowlist that rejects an unrelated test's driver path
+ /// with <c>AdbcStatusCode.Unauthorized</c>.
+ /// </para>
/// </remarks>
public static IDriverAllowlist? Allowlist
{
diff --git
a/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverManagerSecurityCollection.cs
b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverManagerSecurityCollection.cs
new file mode 100644
index 000000000..51021ce93
--- /dev/null
+++
b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverManagerSecurityCollection.cs
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using Xunit;
+
+namespace Apache.Arrow.Adbc.Tests.DriverManager
+{
+ /// <summary>
+ /// Single source of truth for the xUnit collection name shared by every
+ /// test class that touches the process-wide static values on
+ /// <see cref="Apache.Arrow.Adbc.DriverManager.DriverManagerSecurity"/>
+ /// (notably <c>AuditLogger</c> and <c>Allowlist</c>) or that calls
+ /// <see
cref="Apache.Arrow.Adbc.DriverManager.AdbcDriverManager.LoadManagedDriver(string,
string)"/>
+ /// / <c>LoadDriver</c>, whose hot path reads those same static values.
+ /// </summary>
+ /// <remarks>
+ /// <para>
+ /// xUnit parallelizes across distinct test classes by default. Without a
+ /// shared collection, a test in one class that mutates
+ /// <c>DriverManagerSecurity.AuditLogger</c> or <c>Allowlist</c> can race
+ /// with a load-path test in another class, producing flaky
+ /// <c>Assert.Single(logger.Attempts)</c> failures or spurious
+ /// "not permitted by the configured allowlist" exceptions.
+ /// </para>
+ /// <para>
+ /// Apply <c>[Collection(DriverManagerSecurityCollection.Name)]</c> to any
+ /// new test class that either mutates those static values or invokes
+ /// <c>AdbcDriverManager.LoadManagedDriver</c>/<c>LoadDriver</c>.
+ /// </para>
+ /// </remarks>
+ [CollectionDefinition(Name)]
+ public sealed class DriverManagerSecurityCollection
+ {
+ public const string Name = "DriverManagerSecurity";
+ }
+}
diff --git
a/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverManagerSecurityIntegrationTests.cs
b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverManagerSecurityIntegrationTests.cs
index af63bc289..200805f7d 100644
---
a/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverManagerSecurityIntegrationTests.cs
+++
b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverManagerSecurityIntegrationTests.cs
@@ -34,7 +34,7 @@ namespace Apache.Arrow.Adbc.Tests.DriverManager
/// xUnit's collection serialization keeps these tests off the parallel
queue
/// alongside <see cref="DriverManagerSecurityTests"/>.
/// </remarks>
- [Collection("DriverManagerSecurity")]
+ [Collection(DriverManagerSecurityCollection.Name)]
public sealed class DriverManagerSecurityIntegrationTests : IDisposable
{
private readonly List<string> _tempDirs = new List<string>();
diff --git
a/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverManagerSecurityTests.cs
b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverManagerSecurityTests.cs
index 2c836dede..6c319a09e 100644
---
a/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverManagerSecurityTests.cs
+++
b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/DriverManagerSecurityTests.cs
@@ -26,6 +26,18 @@ namespace Apache.Arrow.Adbc.Tests.DriverManager
/// <summary>
/// Tests for <see cref="DriverManagerSecurity"/> functionality.
/// </summary>
+ /// <remarks>
+ /// Mutates process-wide static values on <see
cref="DriverManagerSecurity"/>
+ /// (notably <see cref="DriverManagerSecurity.AuditLogger"/> and
+ /// <see cref="DriverManagerSecurity.Allowlist"/>) and resets them to null
+ /// in <see cref="Dispose"/>. xUnit's collection serialization keeps these
+ /// tests off the parallel queue alongside
+ /// <see cref="DriverManagerSecurityIntegrationTests"/>, which also depends
+ /// on those static values. Without this, a concurrent Dispose here can
null out
+ /// an AuditLogger an integration test just installed, causing
+ /// Assert.Single(logger.Attempts) to see an empty list.
+ /// </remarks>
+ [Collection(DriverManagerSecurityCollection.Name)]
public class DriverManagerSecurityTests : IDisposable
{
private readonly List<string> _tempDirs = new List<string>();
diff --git
a/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/ManagedDriverLoaderTests.cs
b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/ManagedDriverLoaderTests.cs
index 9b9f9875b..2f938239c 100644
---
a/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/ManagedDriverLoaderTests.cs
+++
b/csharp/test/Apache.Arrow.Adbc.Tests/DriverManager/ManagedDriverLoaderTests.cs
@@ -32,6 +32,18 @@ namespace Apache.Arrow.Adbc.Tests.DriverManager
/// is verified on .NET Framework and on modern .NET where dependency
/// resolution goes through <c>AssemblyLoadContext</c>.
/// </summary>
+ /// <remarks>
+ /// Every call here goes through
<c>AdbcDriverManager.LoadManagedDriver</c>,
+ /// which consults the process-wide <see
cref="DriverManagerSecurity.Allowlist"/>
+ /// and <see cref="DriverManagerSecurity.AuditLogger"/>. Other test classes
+ /// (notably <c>DriverManagerSecurityTests</c> and
+ /// <c>DriverManagerSecurityIntegrationTests</c>) mutate those static
values, so
+ /// we join the same xUnit collection to keep them off the parallel queue.
+ /// Without this, a concurrent test that installs a restrictive
+ /// <c>DirectoryAllowlist</c> would cause loads from our private temp
+ /// directory to fail with "not permitted by the configured allowlist".
+ /// </remarks>
+ [Collection(DriverManagerSecurityCollection.Name)]
public class ManagedDriverLoaderTests : IDisposable
{
private readonly List<string> _tempDirs = new List<string>();
