Always show absolute destination in stdout#91
Merged
Conversation
Reviewer's GuideThis PR adds support for always showing absolute (or relative) destinations in stdout by extending the TestCases iterator with a ‘relative’ mode and computing destination paths accordingly, introduces a helper to fetch the working directory, enforces absolute path resolution in the link helper, and updates the directory‐contents assertion to operate on absolute paths. Sequence diagram for absolute/relative destination path handling in TestCasessequenceDiagram
participant TestCases
participant T_testing_T
participant pwd_t
participant testbody
Note over TestCases: For each test, check 'relative' mode
TestCases->>T_testing_T: Create TestCase with Dest
alt relative mode
TestCases->>pwd_t: Get working directory
TestCases->>T_testing_T: Compute relative path for Dest
TestCases->>TestCase: Set Dest to relative path
else absolute mode
TestCases->>TestCase: Dest remains absolute
end
TestCases->>testbody: Run test with TestCase
Class diagram for updated TestCase and Asserter usageclassDiagram
class TestCase {
+string Src
+string Dest
+[]string ExpectedStdout
+map[string]string ExpectedFiles
+Binary ExpectedBinary
+[]snaggle.Option Options
+[]string Flags
}
class Asserter {
+testing.T t
+Testify Testify
+DirectoryContents(ExpectedContents map[string]string, dir string)
}
TestCase --> Binary
Asserter --> Testify
Asserter : DirectoryContents() now resolves dir to absolute path
TestCase : Dest can be absolute or relative depending on test mode
Class diagram for link helper enforcing absolute targetDirclassDiagram
class Link {
+error link(sourcePath string, targetDir string)
+targetDir is resolved to absolute path before use
}
Class diagram for pwd helper functionclassDiagram
class pwd {
+string pwd(t *testing.T)
+calls os.Getwd()
+returns working directory
}
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The nested loops for relative/inplace/recursive in TestCases are duplicated in two code paths—consider refactoring the test-case setup into shared helpers to reduce redundancy and improve readability.
- The pwd helper uses t.Error on Getwd failure but then continues execution, which can lead to confusing downstream errors—switch to t.Fatal to abort immediately on critical failures.
- In DirectoryContents, using t.Error for filepath.Abs failures may allow the test to proceed with an invalid path—fail fast with t.Fatal to avoid masking errors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The nested loops for relative/inplace/recursive in TestCases are duplicated in two code paths—consider refactoring the test-case setup into shared helpers to reduce redundancy and improve readability.
- The pwd helper uses t.Error on Getwd failure but then continues execution, which can lead to confusing downstream errors—switch to t.Fatal to abort immediately on critical failures.
- In DirectoryContents, using t.Error for filepath.Abs failures may allow the test to proceed with an invalid path—fail fast with t.Fatal to avoid masking errors.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #91 +/- ##
==========================================
- Coverage 73.77% 73.39% -0.38%
==========================================
Files 12 12
Lines 610 639 +29
==========================================
+ Hits 450 469 +19
- Misses 112 117 +5
- Partials 48 53 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #89
Summary by Sourcery
Ensure destinations are always shown as absolute paths by normalizing target directories and assertion directories, and by adding a test mode to validate both absolute and relative path handling.
New Features:
Enhancements:
Tests: