Skip to content

chore: Refactor usages of File. and Directory. to use wrappers#3058

Merged
tippmar-nr merged 1 commit intomainfrom
chore/file-and-directory-cleanup
Mar 27, 2025
Merged

chore: Refactor usages of File. and Directory. to use wrappers#3058
tippmar-nr merged 1 commit intomainfrom
chore/file-and-directory-cleanup

Conversation

@tippmar-nr
Copy link
Copy Markdown
Member

  • Refactors usage of File. and Directory. to use IFileWrapper and IDirectoryWrapper instead.
  • Adds methods to IFileWrapper and IDirectoryWrapper as needed with default pass-through implementation.
  • Adds a static Instance property to FileWrapper and DirectoryWrapper for use in static methods
  • Removes IFileReaderWrapper and moves implementation to IFileWrapper and FileWrapper
  • Does not alter any unit tests (aside from required argument refactoring); this change simply reduces friction for future unit tests that may need to mock file and directory operations.
  • Did not attempt to refactor usages of Path. as that usage is ubiquitous throughout the codebase and we haven't previously had a need to mock that functionality for tests.

@tippmar-nr tippmar-nr requested a review from a team as a code owner March 26, 2025 19:33
@tippmar-nr tippmar-nr enabled auto-merge (squash) March 26, 2025 19:33
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 48.64865% with 19 lines in your changes missing coverage. Please review.

Project coverage is 82.22%. Comparing base (8c70ed2) to head (b990bc1).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../NewRelic/Agent/Core/Config/ConfigurationLoader.cs 28.57% 5 Missing ⚠️
...t/NewRelic/Agent/Core/AgentInstallConfiguration.cs 0.00% 3 Missing and 1 partial ⚠️
.../NewRelic/Agent/Core/Logging/LoggerBootstrapper.cs 33.33% 1 Missing and 1 partial ⚠️
...gent/NewRelic/Agent/Core/RuntimeEnvironmentInfo.cs 0.00% 2 Missing ⚠️
.../NewRelic/Agent/Core/Utilities/ExtensionsLoader.cs 0.00% 2 Missing ⚠️
.../Agent/NewRelic/Agent/Core/Utilities/SystemInfo.cs 50.00% 2 Missing ⚠️
.../Configuration/AppSettingsConfigResolveWhenUsed.cs 0.00% 1 Missing ⚠️
...ent/Core/Instrumentation/InstrumentationWatcher.cs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3058      +/-   ##
==========================================
+ Coverage   82.18%   82.22%   +0.04%     
==========================================
  Files         477      477              
  Lines       30623    30625       +2     
  Branches     3426     3426              
==========================================
+ Hits        25166    25180      +14     
+ Misses       4658     4644      -14     
- Partials      799      801       +2     
Flag Coverage Δ
Agent 83.27% <48.64%> (+0.04%) ⬆️
Profiler 72.51% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...wRelic/Agent/Core/Config/BootstrapConfiguration.cs 98.86% <100.00%> (ø)
...NewRelic/Agent/Core/Config/ConfigurationTracker.cs 100.00% <100.00%> (ø)
...ewRelic/Agent/Core/Utilization/UtilizationStore.cs 85.36% <100.00%> (ø)
...gent/NewRelic/Agent/Core/Utilization/VendorInfo.cs 90.00% <100.00%> (+0.86%) ⬆️
...Core/WireModels/LoadedModuleWireModelCollection.cs 89.43% <100.00%> (ø)
.../Configuration/AppSettingsConfigResolveWhenUsed.cs 0.00% <0.00%> (ø)
...ent/Core/Instrumentation/InstrumentationWatcher.cs 33.33% <66.66%> (+1.25%) ⬆️
.../NewRelic/Agent/Core/Logging/LoggerBootstrapper.cs 77.57% <33.33%> (ø)
...gent/NewRelic/Agent/Core/RuntimeEnvironmentInfo.cs 16.84% <0.00%> (ø)
.../NewRelic/Agent/Core/Utilities/ExtensionsLoader.cs 32.87% <0.00%> (ø)
... and 3 more

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nrcventura
Copy link
Copy Markdown
Member

Is there value in making these changes if we do not add unit tests to the code, or do not plan to add unit tests to the code? If we do plan on adding unit tests, and this is just the first step in that process, then I think it is fine. Otherwise, I do not think we should make these changes.

@tippmar-nr tippmar-nr merged commit 1f446c2 into main Mar 27, 2025
96 checks passed
@tippmar-nr tippmar-nr deleted the chore/file-and-directory-cleanup branch March 27, 2025 15:01
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.

4 participants