Skip to content

Provide clear error when directory does not exist #5216

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

johnterickson
Copy link
Contributor

Context

There is lots of confusion from cryptic tar errors for the simple case that the directory to archive does not exist see #12892

tar: could not chdir to 'C:\cargo_target_dir'
...
##[error]Process returned non-zero exit code: 1

Description

This adds a clear error message of when the directory does not exist.


Risk Assessment (Low / Medium / High)

Low - this is already a failure path


Unit Tests Added or Updated (Yes / No)

Yes


Additional Testing Performed

None

@fadnavistanmay fadnavistanmay requested a review from Copilot May 19, 2025 20:29
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

Adds a clear check and error message when the input directory does not exist in the tar‐archiving utility, along with a corresponding unit test.

  • Introduce Directory.Exists validation in TarUtils.ArchiveFilesToTarAsync
  • Throw a DirectoryNotFoundException with a descriptive message for nonexistent paths
  • Add an L0 test to verify the new exception behavior

Reviewed Changes

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

File Description
src/Agent.Plugins/PipelineCache/TarUtils.cs Added a precondition to check for directory existence and throw a clear error
src/Test/L0/Plugin/TarUtilsL0.cs Added an L0 test to assert that a missing directory throws the expected exception
Comments suppressed due to low confidence (2)

src/Test/L0/Plugin/TarUtilsL0.cs:26

  • [nitpick] The test method name is a bit verbose and includes an unnecessary Tar_ prefix. Consider renaming to ArchiveFilesToTarAsync_NonexistentDirectory_ThrowsDirectoryNotFoundException to match standard naming conventions.
public async Task Tar_ArchiveFilesToTarAsync_ThrowsDirectoryNotFoundException()

src/Agent.Plugins/PipelineCache/TarUtils.cs:41

  • [nitpick] The exception message here uses single quotes around the path, whereas the file-path check above does not. For consistency, align the formatting of both messages (either both with quotes or both without).
throw new DirectoryNotFoundException($"Please specify path to a directory, '{inputPath}' does not exist.");

@fadnavistanmay
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@johnterickson johnterickson force-pushed the dev/jerick/dirNotFound branch from 7939b33 to 8e964db Compare May 21, 2025 17:35
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.

3 participants