Skip to content

Conversation

@YakDriver
Copy link
Member

@YakDriver YakDriver commented Sep 8, 2025

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.

  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.

  • If applicable, I've documented the impact of any changes to security controls.

    Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

Overview

This PR modernizes the test infrastructure and CI/CD pipeline to improve cross-platform compatibility and reduce Windows-specific test failures.

Changes Made

1. Split CI/CD Basic Validation for Windows/Linux

  • Before: Single basic-validation job running only on Ubuntu
  • After: Separate linux-basic-validation and windows-basic-validation jobs
  • Benefit: Catch platform-specific issues (formatting, build, unit tests) earlier in the pipeline
  • Both jobs run on PRs and pushes with Go 1.24 & 1.25 matrix
  • Windows job includes proper git config and Windows-specific cache paths

2. Build for 32-bit in CI/CD

  • Ensure builds work in 32-bit environments to avoid this

3. Adapt Tests for Windows Compatibility

  • File getter tests: Added platform-aware symlink testing using runtime.GOOS checks
  • Cross-platform paths: Ensured proper path handling across Unix and Windows systems
  • Tests now skip Windows-incompatible operations while preserving core functionality validation

4. Remove Redundant Temporary Directory Cleanup

  • Removed: Redundant defer func() { _ = os.RemoveAll(dst) }() patterns from 11+ HTTP test functions
  • Benefit: Automatic cleanup, better Windows compatibility, and cleaner test code

5. Add CI/CD check for modern Go code

@YakDriver YakDriver requested a review from a team as a code owner September 8, 2025 17:10
@dduzgun-security dduzgun-security merged commit 4bddabb into hashicorp:main Sep 8, 2025
12 checks passed
@ritikrajdev
Copy link

LGTM!

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.

3 participants