Skip to content

Improvements to filesystem abstractions (part 3) #4498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 16, 2025

Conversation

arturcic
Copy link
Member

This pull request includes several changes to improve the test infrastructure and refactor the code for better maintainability. The most important changes involve adding new dependencies, refactoring the service configuration, and replacing various helper methods with a unified FileSystemHelper class.

Dependency Management:

  • Added System.IO.Abstractions package reference in GitVersion.Common.csproj to facilitate file system abstractions.

Refactoring Service Configuration:

  • Refactored SetUp methods in ArgumentParserOnBuildServerTests and ArgumentParserTests to use GitVersionAppModule for service configuration. [1] [2]

File System Helper Integration:

Code Cleanup:

  • Removed unused PathHelper and DirectoryHelper methods and replaced them with FileSystemHelper in various test assertions and setups. [1] [2] [3] [4] [5]

Additional Imports:

  • Added necessary imports for GitVersion.Extensions and GitVersion.FileSystemGlobbing in relevant files to support the new changes. [1] [2] [3]

Replaces `PathHelper` and `DirectoryHelper` with a centralized `FileSystemHelper` for consistent file system operations. Removes redundant helper implementations and updates all references within the codebase to adopt the new utility. This change improves maintainability and leverages `System.IO.Abstractions` for better testability.
@arturcic arturcic added this to the 6.2.x milestone Apr 16, 2025
@Copilot Copilot AI review requested due to automatic review settings April 16, 2025 07:49
Copy link

@Copilot 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.

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

Files not reviewed (1)
  • new-cli/GitVersion.Common/GitVersion.Common.csproj: Language not supported
Comments suppressed due to low confidence (1)

src/GitVersion.App.Tests/ExecCmdLineArgumentTest.cs:55

  • [nitpick] Verify that using GetTempPathLegacy is intentional; if legacy behavior is not required, standardizing on FileSystemHelper.Path.GetTempPath may improve consistency.
        var workingDirectory = FileSystemHelper.Path.GetTempPathLegacy();

@arturcic arturcic force-pushed the feature/filesystem branch from 49230d8 to 3974f77 Compare April 16, 2025 08:52
@arturcic arturcic requested a review from Copilot April 16, 2025 10:05
Copy link

@Copilot 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

This PR refactors filesystem operations to use a unified FileSystemHelper for better maintainability and improved test infrastructure while also updating dependency management and service configuration.

  • Replaces all usages of PathHelper and DirectoryHelper with FileSystemHelper calls.
  • Refactors service configuration to use GitVersionAppModule for dependency injection.
  • Updates various test files to align with the new filesystem abstraction patterns.

Reviewed Changes

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

Show a summary per file
File Description
src/GitVersion.App/GitVersionExecutor.cs Uses FileSystemHelper for mutex naming and error messages replacing PathHelper calls.
src/GitVersion.App/GitVersionAppModule.cs Adds necessary import for GitVersion.FileSystemGlobbing.
src/GitVersion.App/FileSystemGlobbing/MatcherExtensions.cs Implements globbing extensions using IFileSystem with modern syntax.
src/GitVersion.App/FileSystemGlobbing/IGlobbingResolver.cs Updates namespace to GitVersion.FileSystemGlobbing.
src/GitVersion.App/FileSystemGlobbing/GlobbingResolver.cs Uses new constructor syntax with IFileSystem and integrates with FileSystemHelper usage.
src/GitVersion.App/FileSystemGlobbing/FileSystemInfoGlobbingWrapper.cs Wraps IFileSystemInfo to support file system globbing.
src/GitVersion.App/FileSystemGlobbing/FileInfoGlobbingWrapper.cs Wraps IFileInfo correctly with FileSystemHelper integration.
src/GitVersion.App/FileSystemGlobbing/DirectoryInfoGlobbingWrapper.cs Implements directory info abstraction using IFileSystem with a minor potential inconsistency.
src/GitVersion.App/FileAppender.cs Replaces PathHelper with FileSystemHelper for log file operations.
src/GitVersion.App/ArgumentParserExtensions.cs Consistently utilizes FileSystemHelper for path operations.
src/GitVersion.App/ArgumentParser.cs Updates configuration file and argument parsing to use FileSystemHelper.
Test Files (e.g., UpdateWixVersionFileTests.cs, TagCheckoutInBuildAgentTests.cs, PullRequestInBuildAgentTest.cs, etc.) Replace legacy helper calls with FileSystemHelper to ensure consistent filesystem access.
src/GitVersion.App.Tests/Helpers/GitVersionHelper.cs Updates log file operations and current directory retrieval using FileSystemHelper.
src/GitVersion.App.Tests/ExecCmdLineArgumentTest.cs Updates temporary path retrieval to use the new FileSystemHelper API.
src/GitVersion.App.Tests/ArgumentParserTests.cs Ensures test assertions reference FileSystemHelper for consistent filename and path handling.
src/GitVersion.App.Tests/ArgumentParserOnBuildServerTests.cs Updates service registration to use GitVersionAppModule.
Files not reviewed (1)
  • new-cli/GitVersion.Common/GitVersion.Common.csproj: Language not supported

@arturcic arturcic force-pushed the feature/filesystem branch from 3974f77 to 9720330 Compare April 16, 2025 10:12
@arturcic arturcic requested a review from Copilot April 16, 2025 10:12
Copy link

@Copilot 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.

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

Files not reviewed (1)
  • new-cli/GitVersion.Common/GitVersion.Common.csproj: Language not supported

Reorganized globbing-related logic into the FileSystemGlobbing namespace. Introduced IFileSystem integration in GlobbingResolver and new wrapper classes for directory and file handling. Simplified service configuration using GitVersionAppModule.
@arturcic arturcic force-pushed the feature/filesystem branch from 9720330 to ad6f152 Compare April 16, 2025 10:15
@arturcic arturcic requested a review from Copilot April 16, 2025 10:24
Copy link

@Copilot 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.

Copilot reviewed 75 out of 76 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • new-cli/GitVersion.Common/GitVersion.Common.csproj: Language not supported

@arturcic arturcic merged commit 5b8828d into GitTools:main Apr 16, 2025
93 checks passed
Copy link
Contributor

mergify bot commented Apr 16, 2025

Thank you @arturcic for your contribution!

@arturcic arturcic deleted the feature/filesystem branch April 16, 2025 14:25
@gittools-bot
Copy link

🎉 This issue has been resolved in version 6.3.0 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants