diff --git a/file.go b/file.go index 73f2fb0..48b0f39 100644 --- a/file.go +++ b/file.go @@ -151,14 +151,17 @@ func MakeGitHubZipFileRequest(gitHubCommit GitHubCommit, gitHubToken string, ins // This represents either a commit, branch, or git tag var gitRef string - if gitHubCommit.GitRef != "" { - gitRef = gitHubCommit.GitRef - } else if gitHubCommit.CommitSha != "" { + // Ordering matters in this conditional + // GitRef needs to be the fallback and therefore must be last + // See https://github.com/gruntwork-io/fetch/issues/87 for an example + if gitHubCommit.CommitSha != "" { gitRef = gitHubCommit.CommitSha } else if gitHubCommit.BranchName != "" { gitRef = gitHubCommit.BranchName } else if gitHubCommit.GitTag != "" { gitRef = gitHubCommit.GitTag + } else if gitHubCommit.GitRef != "" { + gitRef = gitHubCommit.GitRef } else { return request, fmt.Errorf("Neither a GitCommitSha nor a GitTag nor a BranchName were specified so impossible to identify a specific commit to download.") } diff --git a/integration_test.go b/integration_test.go new file mode 100644 index 0000000..02b6c71 --- /dev/null +++ b/integration_test.go @@ -0,0 +1,145 @@ +package main + +import ( + "bytes" + "fmt" + "io" + "io/ioutil" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + cli "gopkg.in/urfave/cli.v1" +) + +func TestFetchWithBranchOption(t *testing.T) { + // Note: we don't run these tests in parallel as runFetchCommandWithOutput currently overrides the + // stdout and stderr variables which may result in unstable tests. + + tmpDownloadPath := createTempDir(t, "fetch-branch-test") + + cases := []struct { + name string + repoUrl string + branchName string + sourcePath string + expectedFile string + }{ + // Test on a public repo whose sole purpose is to be a test fixture for this tool + {"branch option with public repo", "https://github.com/gruntwork-io/fetch-test-public", "sample-branch", "/", "foo.txt"}, + + // Private repo equivalent + {"branch option with private repo", "https://github.com/gruntwork-io/fetch-test-private", "sample-branch", "/", "bar.txt"}, + } + + for _, tc := range cases { + // The following is necessary to make sure tc's values don't + // get updated due to concurrency within the scope of t.Run(..) below + tc := tc + + t.Run(tc.name, func(t *testing.T) { + cmd := fmt.Sprintf("fetch --repo %s --branch %s --source-path %s %s", tc.repoUrl, tc.branchName, tc.sourcePath, tmpDownloadPath) + output, _, err := runFetchCommandWithOutput(t, cmd) + require.NoError(t, err) + + // When --branch is specified, ensure the latest commit is fetched + assert.Contains(t, output, "Downloading latest commit from branch") + + // Ensure the expected file was downloaded + assert.FileExists(t, JoinPath(tmpDownloadPath, tc.expectedFile)) + }) + } +} + +func runFetchCommandWithOutput(t *testing.T, command string) (string, string, error) { + // Note: As most of fetch writes directly to stdout and stderr using the fmt package, we need to temporarily override + // the OS pipes. This is based loosely on https://stackoverflow.com/questions/10473800/in-go-how-do-i-capture-stdout-of-a-function-into-a-string/10476304#10476304 + // Our goal eventually is to remove this, but we'll need to introduce a logger as mentioned in: https://github.com/gruntwork-io/fetch/issues/89 + stdout := bytes.Buffer{} + stderr := bytes.Buffer{} + + stdoutReader, stdoutWriter, err1 := os.Pipe() + if err1 != nil { + return "", "", err1 + } + + stderrReader, stderrWriter, err2 := os.Pipe() + if err2 != nil { + return "", "", err2 + } + + oldStdout := os.Stdout + oldStderr := os.Stderr + + // override the pipes to capture output + os.Stdout = stdoutWriter + os.Stderr = stderrWriter + + // execute the fetch command which produces output + err := runFetchCommand(t, command) + if err != nil { + return "", "", err + } + + // copy the output to the buffers in separate goroutines so printing can't block indefinitely + stdoutC := make(chan bytes.Buffer) + stderrC := make(chan bytes.Buffer) + go func() { + var buf bytes.Buffer + io.Copy(&buf, stdoutReader) + stdoutC <- buf + }() + + go func() { + var buf bytes.Buffer + io.Copy(&buf, stderrReader) + stderrC <- buf + }() + + // reset the pipes back to normal + stdoutWriter.Close() + stderrWriter.Close() + os.Stdout = oldStdout + os.Stderr = oldStderr + stdout = <-stdoutC + stderr = <-stderrC + + // log the buffers for easier debugging. this is inspired by the integration tests in Terragrunt. + // For more information, see: https://github.com/gruntwork-io/terragrunt/blob/master/test/integration_test.go. + logBufferContentsLineByLine(t, stdout, "stdout") + logBufferContentsLineByLine(t, stderr, "stderr") + return stdout.String(), stderr.String(), nil +} + +func runFetchCommand(t *testing.T, command string) error { + args := strings.Split(command, " ") + + app := CreateFetchCli(VERSION) + app.Action = runFetchWrapper + return app.Run(args) +} + +func logBufferContentsLineByLine(t *testing.T, out bytes.Buffer, label string) { + t.Logf("[%s] Full contents of %s:", t.Name(), label) + lines := strings.Split(out.String(), "\n") + for _, line := range lines { + t.Logf("[%s] %s", t.Name(), line) + } +} + +func createTempDir(t *testing.T, prefix string) string { + dir, err := ioutil.TempDir(os.TempDir(), prefix) + if err != nil { + t.Fatalf("Could not create temporary directory due to error: %v", err) + } + defer os.RemoveAll(dir) + return dir +} + +// We want to call runFetch() using the app.Action wrapper like the main CLI handler, but we don't want to write to strerr +// and suddenly exit using os.Exit(1), so we use a separate wrapper method in the integration tests. +func runFetchTestWrapper(c *cli.Context) error { + return runFetch(c) +} diff --git a/main.go b/main.go index 1835213..819cb23 100644 --- a/main.go +++ b/main.go @@ -52,12 +52,19 @@ const optionWithProgress = "progress" const envVarGithubToken = "GITHUB_OAUTH_TOKEN" -func main() { +// Create the Fetch CLI App +func CreateFetchCli(version string) *cli.App { + cli.OsExiter = func(exitCode int) { + // Do nothing. We just need to override this function, as the default value calls os.Exit, which + // kills the app (or any automated test) dead in its tracks. + } + app := cli.NewApp() app.Name = "fetch" app.Usage = "fetch makes it easy to download files, folders, and release assets from a specific git commit, branch, or tag of public and private GitHub repos." app.UsageText = "fetch [global options] \n (See https://github.com/gruntwork-io/fetch for examples, argument definitions, and additional docs.)" - app.Version = VERSION + app.Author = "Gruntwork " + app.Version = version app.Flags = []cli.Flag{ cli.StringFlag{ @@ -112,6 +119,11 @@ func main() { }, } + return app +} + +func main() { + app := CreateFetchCli(VERSION) app.Action = runFetchWrapper // Run the definition of App.Action @@ -290,14 +302,17 @@ func downloadSourcePaths(sourcePaths []string, destPath string, githubRepo GitHu // Download that release as a .zip file - if gitHubCommit.GitRef != "" { - fmt.Printf("Downloading git reference \"%s\" of %s ...\n", gitHubCommit.GitRef, githubRepo.Url) - } else if gitHubCommit.CommitSha != "" { + // Ordering matters in this conditional + // GitRef needs to be the fallback and therefore must be last + // See https://github.com/gruntwork-io/fetch/issues/87 for an example + if gitHubCommit.CommitSha != "" { fmt.Printf("Downloading git commit \"%s\" of %s ...\n", gitHubCommit.CommitSha, githubRepo.Url) } else if gitHubCommit.BranchName != "" { fmt.Printf("Downloading latest commit from branch \"%s\" of %s ...\n", gitHubCommit.BranchName, githubRepo.Url) } else if gitHubCommit.GitTag != "" { fmt.Printf("Downloading tag \"%s\" of %s ...\n", latestTag, githubRepo.Url) + } else if gitHubCommit.GitRef != "" { + fmt.Printf("Downloading git reference \"%s\" of %s ...\n", gitHubCommit.GitRef, githubRepo.Url) } else { return fmt.Errorf("The commit sha, tag, and branch name are all empty.") } diff --git a/util.go b/util.go new file mode 100644 index 0000000..9a515ed --- /dev/null +++ b/util.go @@ -0,0 +1,10 @@ +package main + +import "path/filepath" + +// Windows systems use \ as the path separator *nix uses / +// Use this function when joining paths to force the returned path to use / as the path separator +// This will improve cross-platform compatibility +func JoinPath(elem ...string) string { + return filepath.ToSlash(filepath.Join(elem...)) +}