Skip to content

Add Windows CI testing#1935

Open
vbabanin wants to merge 8 commits intomongodb:mainfrom
vbabanin:JAVA-6057
Open

Add Windows CI testing#1935
vbabanin wants to merge 8 commits intomongodb:mainfrom
vbabanin:JAVA-6057

Conversation

@vbabanin
Copy link
Copy Markdown
Member

@vbabanin vbabanin commented Apr 9, 2026

Changes in this PR:

  • Add Windows 2022 Evergreen build variants for server 8.2 across all topologies to enable Windows CI coverage
  • Add EnvironmentProvider class to centralize all System.getenv() calls, replacing the reflection-based env var override hack that fails on Windows due to JDK's ProcessEnvironment dual-map architecture
  • Fix SSL keystore/truststore paths in Evergreen scripts to use native Windows paths via cygpath
  • Fix JSON test file path separator handling and mongocryptd temp directory for Windows compatibility
  • Skip Unix-domain-socket test on Windows

JAVA-6057

Checklist

  • Jira ticket linked in the description
  • AI-generated code reviewed by author antagonistically
  • AI review completed on draft PR (Copilot or/and Claude)
  • Comments added for complex/confusing areas
  • PR is appropriately scoped
  • Manually triggered test variants not covered by PR automation, if applicable (e.g., Atlas, OIDC)

AI Usage & Effectiveness

What Claude (Opus 4.6) did (mostly boilerplate/mechanical refactoring):

  • Wrote the EnvironmentProvider class (after the author, without Claude, designed a detailed plan using the lambda-based approach)
  • Migrated all System.getenv() call sites to EnvironmentProvider.getEnv() (FaasEnvironment, OidcAuthenticator, BuiltInAwsCredentialSupplier, TracingManager, MicrometerTracer)
  • Removed FaasEnvironmentAccessor and the reflection hack from AbstractMicrometerProseTest
  • Updated the WithWrapper class to use the new pattern
  • Fixed the mongocryptd /tmp/ path for Windows
  • [Experiment] Used Evergreen MCP via Claude as an experiment to retrieve logs and provide a quick summary of the failure. Results: Claude was slower to retrieve those logs than manually downloading them. However, Claude was useful for analyzing the failures sometimes.

What Claude (Opus 4.6) suggestions were rejected:

  • Rejected multiple Claude proposals for EnvironmentProvider (volatile, ConcurrentHashMap, override(Map) constructor)

What was done manually:

  • Submitted Evergreen patches and iterated on failures during troubleshooting
  • Designed the EnvironmentProvider approach (lambda field, no volatile, no ConcurrentHashMap)
  • Updated shell scripts
  • Chose server version 8.2 for Windows (based on SERVER-116018)
  • Unit tests

- Add Windows 2022 Evergreen build variants for server 8.2 across all topologies to enable Windows CI coverage
- Add EnvironmentProvider class to centralize all System.getenv() calls, replacing the reflection-based env var override hack that fails on Windows due to JDK's ProcessEnvironment dual-map architecture
- Fix SSL keystore/truststore paths in Evergreen scripts to use native Windows paths via cygpath
- Fix JSON test file path separator handling and mongocryptd temp directory for Windows compatibility
- Skip Unix-domain-socket test on Windows

JAVA-6057
@vbabanin vbabanin self-assigned this Apr 9, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Windows CI coverage and improves Windows compatibility across tests and Evergreen configuration by standardizing environment-variable access and fixing platform-specific path handling.

Changes:

  • Add Windows 2022 Evergreen build variants (server 8.2) and Windows-specific path handling in Evergreen scripts.
  • Introduce EnvironmentProvider to centralize env var access and enable test-time overrides without reflection hacks.
  • Fix Windows compatibility issues in tests (temp dirs, path separators) and skip a non-portable socket test on Windows.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
driver-sync/src/test/functional/com/mongodb/client/AbstractSessionsProseTest.java Write mongocryptd logs to Java temp dir for cross-platform support.
driver-sync/src/test/functional/com/mongodb/client/AbstractMicrometerProseTest.java Replace reflective env-var mutation with EnvironmentProvider overrides in micrometer prose tests.
driver-core/src/test/unit/com/mongodb/internal/EnvironmentProviderTest.java Add unit tests for env lookup/override behavior.
driver-core/src/test/functional/com/mongodb/internal/connection/SocketStreamHelperSpecification.groovy Skip a socket configuration test on Windows.
driver-core/src/test/functional/com/mongodb/internal/connection/FaasEnvironmentAccessor.java Remove test-only accessor for env overrides (superseded by EnvironmentProvider).
driver-core/src/test/functional/com/mongodb/internal/connection/ClientMetadataTest.java Update .dockerenv path to match production logic and be OS-correct.
driver-core/src/test/functional/com/mongodb/client/WithWrapper.java Switch env-var test wrapper to EnvironmentProvider.envOverride().
driver-core/src/main/com/mongodb/internal/observability/micrometer/TracingManager.java Use EnvironmentProvider.getEnv for observability env vars.
driver-core/src/main/com/mongodb/internal/observability/micrometer/MicrometerTracer.java Use EnvironmentProvider.getEnv for query text length env var.
driver-core/src/main/com/mongodb/internal/EnvironmentProvider.java New centralized env var provider + override mechanism for tests.
driver-core/src/main/com/mongodb/internal/connection/OidcAuthenticator.java Read OIDC-related env vars via EnvironmentProvider.
driver-core/src/main/com/mongodb/internal/connection/FaasEnvironment.java Delegate env var reads to EnvironmentProvider (removing test override map).
driver-core/src/main/com/mongodb/internal/authentication/BuiltInAwsCredentialSupplier.java Read AWS env vars via EnvironmentProvider.
bson/src/test/unit/util/JsonPoweredTestHelper.java Normalize path separators to reliably locate JSON spec files on Windows.
.evergreen/setup-env.bash Add Windows-specific JDK paths for Evergreen.
.evergreen/run-tests.sh Use native Windows paths for SSL keystore/truststore and log Windows version.
.evergreen/run-csfle-tests-with-mongocryptd.sh Use native Windows paths for SSL keystore/truststore.
.evergreen/.evg.yml Add Windows axes/buildvariants and introduce MongoDB 8.2 for Windows coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread driver-core/src/main/com/mongodb/internal/EnvironmentProvider.java
Comment thread driver-core/src/main/com/mongodb/internal/EnvironmentProvider.java
Comment thread .evergreen/setup-env.bash
Comment thread .evergreen/.evg.yml
vbabanin added 2 commits May 6, 2026 17:21
# Conflicts:
#	driver-core/src/main/com/mongodb/internal/observability/micrometer/TracingManager.java
#	driver-sync/src/test/functional/com/mongodb/client/AbstractMicrometerProseTest.java
@vbabanin vbabanin requested a review from Copilot May 7, 2026 01:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Comment thread .evergreen/.evg.yml
Comment thread .evergreen/.evg.yml
tags: [ "tests-variant" ]
tasks:
- name: "test-sync-task"
exec_timeout_secs: 7200
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests run longer on Windows ~1h 30min - 1h 40 min, so it is bumped to 2 hours.

*
* <p>Tests can override values via {@link #envOverride()}.</p>
*/
public final class EnvironmentProvider {
Copy link
Copy Markdown
Member Author

@vbabanin vbabanin May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this PR introduces EnvironmentProvider (and consolidates env handling)

We previously used a reflection-based test helper in AbstractMucromiterProseTests to override environment variables. That approach broke on Windows due to differences in the JDK’s ProcessEnvironment implementation.

What worked on Linux/macOS

ProcessEnvironment effectively exposes a single backing map. The reflection helper mutates that map, and System.getenv() / System.getenv("KEY") read from the same data, thus overrides are visible.

What breaks on Windows

The JDK maintains two maps:

  • theEnvironment (HashMap): backs System.getenv() (returns the full map)
  • theCaseInsensitiveEnvironment (TreeMap, without case-insensitive comparator): backs System.getenv(String key) (single-key lookup)

The reflection helper only updated theEnvironment. Our production code uses System.getenv("OBSERVABILITY_MONGODB_ENABLED"), which reads from theCaseInsensitiveEnvironment, so the override was invisible on Windows, thus Micrometer tests failed.

What this PR changes

Introduces EnvironmentProvider to avoid reflection/JDK internals entirely. Production code calls EnvironmentProvider.getEnv() instead of System.getenv().

Tests override the provider via envOverride(...) (swapping the underlying supplier/lambda), which behaves consistently across platforms.

EnviromentOverride is AutoCloseable, so tests can use try-with-resources and avoid boilerplate while keeping overrides scoped and explicit.

@vbabanin vbabanin requested a review from Copilot May 7, 2026 03:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

@vbabanin vbabanin requested review from a team, stIncMale and strogiyotec and removed request for a team and stIncMale May 7, 2026 04:46
@vbabanin vbabanin marked this pull request as ready for review May 7, 2026 05:14
@vbabanin vbabanin requested a review from a team as a code owner May 7, 2026 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants