-
Notifications
You must be signed in to change notification settings - Fork 1k
Conversation
This sounds fine. I'd also use figuring this out as a reason to look at some of the other tests to see how they accomplish this goal. You'd also likely be able to reuse some of the testing harness internals to facilitate this, even though doing a standard case on disk doesn't work. |
@sdboyer : i have added the test for checking relative path without touching the existing tests. The tests check if dep init ran correctly and created the .toml and .lock files correctly. |
if !filepath.IsAbs(args[0]) { | ||
root = filepath.Join(ctx.WorkingDir, args[0]) | ||
} | ||
if err := os.MkdirAll(root, os.FileMode(0777)); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the default 777? Wouldn't 755 be enough for most cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had made it 755 in my previous PR, i got a comment to change it to 777.
#549 (comment)
// below the wd, and therefore the GOPATH, is recorded as "/var/..." but the | ||
// actual process runs in "/private/var/..." and dies due to not being in the | ||
// GOPATH because the roots don't line up. | ||
if externalProc && runtime.GOOS == "darwin" && needsPrivateLeader(new.tempdir) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than duplicating all the code + comment, let's please extract it out into a func and use that in both places.
OK, LGTM. Thank you, and yay first contribution! 🎉 🎉 |
Fixes #537
Need some help on test cases. The current approach is giving issues as adding tests to
harness_tests/init
cannot find vendor paths. Let me know if there is a better approach to test this feature.