Skip to content

Commit

Permalink
Pete/87/broken branch (#88)
Browse files Browse the repository at this point in the history
* Fix broken --branch regression

* Change ordering inside MakeGitHubZipFileRequest

* add initial integration tests to fetch

* address pr feedback

* Adding comments that conditional ordering matters

Co-authored-by: Rob Morgan <[email protected]>
  • Loading branch information
pete0emerson and robmorgan authored Feb 3, 2021
1 parent c6fdad3 commit c97d990
Show file tree
Hide file tree
Showing 4 changed files with 181 additions and 8 deletions.
9 changes: 6 additions & 3 deletions file.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
}
Expand Down
145 changes: 145 additions & 0 deletions integration_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
25 changes: 20 additions & 5 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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] <local-download-path>\n (See https://github.com/gruntwork-io/fetch for examples, argument definitions, and additional docs.)"
app.Version = VERSION
app.Author = "Gruntwork <www.gruntwork.io>"
app.Version = version

app.Flags = []cli.Flag{
cli.StringFlag{
Expand Down Expand Up @@ -112,6 +119,11 @@ func main() {
},
}

return app
}

func main() {
app := CreateFetchCli(VERSION)
app.Action = runFetchWrapper

// Run the definition of App.Action
Expand Down Expand Up @@ -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.")
}
Expand Down
10 changes: 10 additions & 0 deletions util.go
Original file line number Diff line number Diff line change
@@ -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...))
}

0 comments on commit c97d990

Please sign in to comment.