From 807bb3e1b2d4a77a71b7fc45f3b238f3515a1e12 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Mon, 26 Oct 2015 16:25:31 -0600 Subject: [PATCH 1/4] rewrite test runner in go to reduce output race conditions --- script/integration | 14 +----- script/integration.go | 113 ++++++++++++++++++++++++++++++++++++++++++ script/script.go | 2 + test/testhelpers.sh | 5 +- test/testlib.sh | 2 +- 5 files changed, 120 insertions(+), 16 deletions(-) create mode 100644 script/integration.go diff --git a/script/integration b/script/integration index 96153a9d..7d40eee5 100755 --- a/script/integration +++ b/script/integration @@ -5,7 +5,6 @@ set -e SHUTDOWN_LFS=no SHOW_LOGS=yes -TESTS=( "$@" ) atexit() { res=${1:-$?} @@ -47,15 +46,4 @@ parallel=${GIT_LFS_TEST_MAXPROCS:-4} echo "Running this maxprocs=$parallel" echo -if [ ${#TESTS[@]} -eq 0 ] -then - testfiles=(test/test-*.sh) -else - for ((i=0; i<${#TESTS[@]}; i++)); do - testfiles[i]=test/test-${TESTS[i]}.sh - done -fi - -for file in "${testfiles[@]}"; do - echo "0$(cat .$(basename $file).time 2>/dev/null || true) $file" -done | sort -rnk1 | awk '{ print $2 }' | xargs -I % -P $parallel -n 1 /bin/bash % --batch +GIT_LFS_TEST_MAXPROCS=$parallel GIT_LFS_TEST_DIR="$GIT_LFS_TEST_DIR" SHUTDOWN_LFS="no" go run script/*.go -cmd integration "$@" diff --git a/script/integration.go b/script/integration.go new file mode 100644 index 00000000..a1d8a222 --- /dev/null +++ b/script/integration.go @@ -0,0 +1,113 @@ +package main + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + "sync" +) + +var ( + showlogs = false + maxprocs = 4 + testPattern = regexp.MustCompile(`test/test-([a-z\-]+)\.sh`) +) + +func mainIntegration() { + if maxprocs < 1 { + maxprocs = 1 + } + + files := testFiles() + + if len(files) == 0 { + fmt.Println("no tests to run") + os.Exit(1) + } + + var wg sync.WaitGroup + tests := make(chan string, len(files)) + output := make(chan string, len(files)) + + for _, file := range files { + tests <- file + } + + go printOutput(output) + for i := 0; i < maxprocs; i++ { + wg.Add(1) + go worker(tests, output, &wg) + } + + close(tests) + wg.Wait() + close(output) + printOutput(output) +} + +func runTest(output chan string, test string) { + out, _ := exec.Command("/bin/bash", test).CombinedOutput() + output <- strings.TrimSpace(string(out)) +} + +func printOutput(output <-chan string) { + for { + select { + case out, ok := <-output: + if !ok { + return + } + + fmt.Println(out) + } + } +} + +func worker(tests <-chan string, output chan string, wg *sync.WaitGroup) { + defer wg.Done() + for { + select { + case testname, ok := <-tests: + if !ok { + return + } + runTest(output, testname) + } + } +} + +func testFiles() []string { + if len(os.Args) < 4 { + return allTestFiles() + } + + fileMap := make(map[string]bool) + for _, file := range allTestFiles() { + fileMap[file] = true + } + + files := make([]string, 0, len(os.Args)-3) + for _, arg := range os.Args { + fullname := "test/test-" + arg + ".sh" + if fileMap[fullname] { + files = append(files, fullname) + } + } + + return files +} + +func allTestFiles() []string { + files := make([]string, 0, 100) + filepath.Walk("test", func(path string, info os.FileInfo, err error) error { + if err != nil || info.IsDir() || !testPattern.MatchString(path) { + return nil + } + files = append(files, path) + return nil + }) + return files +} diff --git a/script/script.go b/script/script.go index 91f966b5..f3e19026 100644 --- a/script/script.go +++ b/script/script.go @@ -19,6 +19,8 @@ func main() { mainBuild() case "release": mainRelease() + case "integration": + mainIntegration() default: log.Fatalln("Unknown command:", *SubCommand) } diff --git a/test/testhelpers.sh b/test/testhelpers.sh index 60af0cac..672f3111 100644 --- a/test/testhelpers.sh +++ b/test/testhelpers.sh @@ -227,6 +227,7 @@ setup() { echo echo "HOME: $HOME" echo "TMP: $TMPDIR" + echo "CREDS: $CREDSDIR" echo "lfstest-gitserver:" echo " LFSTEST_URL=$LFS_URL_FILE" echo " LFSTEST_DIR=$REMOTEDIR" @@ -408,7 +409,7 @@ escape_path() { # As native_path but escape all backslash characters to "\\" native_path_escaped() { local unescaped=$(native_path "$1") - escape_path "$unescaped" + escape_path "$unescaped" } # Compare 2 lists which are newline-delimited in a string, ignoring ordering and blank lines @@ -429,4 +430,4 @@ contains_same_elements() { is_stdin_attached() { test -t0 echo $? -} \ No newline at end of file +} diff --git a/test/testlib.sh b/test/testlib.sh index a457c98b..aab07984 100644 --- a/test/testlib.sh +++ b/test/testlib.sh @@ -75,7 +75,7 @@ begin_test () { err="$TRASHDIR/err" trace="$TRASHDIR/trace" - exec 1>"$out" 2>"$err" + exec 1>"$out" 2>"$err" # enabling GIT_TRACE can cause Windows git to stall, esp with fd 5 # other fd numbers like 8/9 don't stall but still don't work, so disable From 47d167feea962c6de22662df21969bd4ae3b1649 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Mon, 26 Oct 2015 16:30:36 -0600 Subject: [PATCH 2/4] return exit code 1 if any of the tests fail --- script/integration.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/script/integration.go b/script/integration.go index a1d8a222..442206f0 100644 --- a/script/integration.go +++ b/script/integration.go @@ -11,7 +11,7 @@ import ( ) var ( - showlogs = false + erroring = false maxprocs = 4 testPattern = regexp.MustCompile(`test/test-([a-z\-]+)\.sh`) ) @@ -46,10 +46,18 @@ func mainIntegration() { wg.Wait() close(output) printOutput(output) + + if erroring { + os.Exit(1) + } } func runTest(output chan string, test string) { - out, _ := exec.Command("/bin/bash", test).CombinedOutput() + out, err := exec.Command("/bin/bash", test).CombinedOutput() + if err != nil { + erroring = true + } + output <- strings.TrimSpace(string(out)) } From da3ced10b9e4b43b344836530b1b0ade9f8bea80 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Mon, 26 Oct 2015 17:11:48 -0600 Subject: [PATCH 3/4] don't format go files in these non-src dirs --- script/fmt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/fmt b/script/fmt index 32780844..e0ac0921 100755 --- a/script/fmt +++ b/script/fmt @@ -6,7 +6,7 @@ hash goimports 2>/dev/null && { } # don't run gofmt in these directories -ignored=(/bin/ /docs/ /log/ /man/ /tmp/ /vendor/) +ignored=(/bin/ /docs/ /log/ /man/ /tmp/ /vendor/ /rpm/ /docker/ /debian/) for i in */ ; do if [[ ! ${ignored[*]} =~ "/$i" ]]; then $formatter -w -l "$@" "${i%?}" From cbafdbc7dc9c1e2e8463e3bf6da0c2633eb096b8 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Mon, 26 Oct 2015 17:12:15 -0600 Subject: [PATCH 4/4] update tests so they pass if GIT_LFS_TEST_DIR is unset --- test/test-init.sh | 35 +++++++++++++++++++---------------- test/test-uninit.sh | 11 +++++++---- test/test-update.sh | 2 +- test/testhelpers.sh | 8 +++++++- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/test/test-init.sh b/test/test-init.sh index 064e017d..c68cdc46 100755 --- a/test/test-init.sh +++ b/test/test-init.sh @@ -6,13 +6,16 @@ begin_test "init again" ( set -e - [ "git-lfs smudge %f" = "$(git config filter.lfs.smudge)" ] - [ "git-lfs clean %f" = "$(git config filter.lfs.clean)" ] + smudge="$(git config filter.lfs.smudge)" + clean="$(git config filter.lfs.clean)" + + printf "$smudge" | grep "git-lfs smudge" + printf "$clean" | grep "git-lfs clean" git lfs init - [ "git-lfs smudge %f" = "$(git config filter.lfs.smudge)" ] - [ "git-lfs clean %f" = "$(git config filter.lfs.clean)" ] + [ "$smudge" = "$(git config filter.lfs.smudge)" ] + [ "$clean" = "$(git config filter.lfs.clean)" ] ) end_test @@ -34,12 +37,12 @@ begin_test "init with old settings" grep -E "(clean|smudge) attribute should be" init.log [ `grep -c "(MISSING)" init.log` = "0" ] - [ "git lfs smudge %f" = "$(git config filter.lfs.smudge)" ] - [ "git lfs clean %f" = "$(git config filter.lfs.clean)" ] + [ "git lfs smudge %f" = "$(git config --global filter.lfs.smudge)" ] + [ "git lfs clean %f" = "$(git config --global filter.lfs.clean)" ] git lfs init --force - [ "git-lfs smudge %f" = "$(git config filter.lfs.smudge)" ] - [ "git-lfs clean %f" = "$(git config filter.lfs.clean)" ] + [ "git-lfs smudge %f" = "$(git config --global filter.lfs.smudge)" ] + [ "git-lfs clean %f" = "$(git config --global filter.lfs.clean)" ] ) end_test @@ -85,7 +88,7 @@ Git LFS initialized." Git LFS initialized." = "$(git lfs init --force)" ] [ "$pre_push_hook" = "$(cat .git/hooks/pre-push)" ] - [ -n "$LFS_DOCKER" ] && exit 0 + has_test_dir || exit 0 echo "test with bare repository" cd .. @@ -127,16 +130,16 @@ begin_test "init --skip-smudge" set -e git lfs init - [ "git-lfs clean %f" = "$(git config filter.lfs.clean)" ] - [ "git-lfs smudge %f" = "$(git config filter.lfs.smudge)" ] + [ "git-lfs clean %f" = "$(git config --global filter.lfs.clean)" ] + [ "git-lfs smudge %f" = "$(git config --global filter.lfs.smudge)" ] git lfs init --skip-smudge - [ "git-lfs clean %f" = "$(git config filter.lfs.clean)" ] - [ "git-lfs smudge --skip %f" = "$(git config filter.lfs.smudge)" ] + [ "git-lfs clean %f" = "$(git config --global filter.lfs.clean)" ] + [ "git-lfs smudge --skip %f" = "$(git config --global filter.lfs.smudge)" ] git lfs init --force - [ "git-lfs clean %f" = "$(git config filter.lfs.clean)" ] - [ "git-lfs smudge %f" = "$(git config filter.lfs.smudge)" ] + [ "git-lfs clean %f" = "$(git config --global filter.lfs.clean)" ] + [ "git-lfs smudge %f" = "$(git config --global filter.lfs.smudge)" ] ) end_test @@ -169,7 +172,7 @@ begin_test "init --local outside repository" set +e - [ -n "$LFS_DOCKER" ] && exit 0 + has_test_dir || exit 0 git lfs init --local 2> err.log res=$? diff --git a/test/test-uninit.sh b/test/test-uninit.sh index bba1f2e2..c4c56a87 100755 --- a/test/test-uninit.sh +++ b/test/test-uninit.sh @@ -6,8 +6,11 @@ begin_test "uninit outside repository" ( set -e - [ "git-lfs smudge %f" = "$(git config filter.lfs.smudge)" ] - [ "git-lfs clean %f" = "$(git config filter.lfs.clean)" ] + smudge="$(git config filter.lfs.smudge)" + clean="$(git config filter.lfs.clean)" + + printf "$smudge" | grep "git-lfs smudge" + printf "$clean" | grep "git-lfs clean" # uninit multiple times to trigger https://github.com/github/git-lfs/issues/529 git lfs uninit @@ -15,8 +18,8 @@ begin_test "uninit outside repository" git lfs uninit | tee uninit.log grep "configuration has been removed" uninit.log - [ "" = "$(git config filter.lfs.smudge)" ] - [ "" = "$(git config filter.lfs.clean)" ] + [ "" = "$(git config --global filter.lfs.smudge)" ] + [ "" = "$(git config --global filter.lfs.clean)" ] cat $HOME/.gitconfig [ "$(grep 'filter "lfs"' $HOME/.gitconfig -c)" = "0" ] diff --git a/test/test-update.sh b/test/test-update.sh index 2587fc82..c7d660e7 100755 --- a/test/test-update.sh +++ b/test/test-update.sh @@ -68,7 +68,7 @@ Run \`git lfs update --force\` to overwrite this hook." [ "Updated pre-push hook." = "$(git lfs update --force)" ] [ "$pre_push_hook" = "$(cat .git/hooks/pre-push)" ] - [ -n "$LFS_DOCKER" ] && exit 0 + has_test_dir || exit 0 echo "test with bare repository" cd .. diff --git a/test/testhelpers.sh b/test/testhelpers.sh index 672f3111..344e070d 100644 --- a/test/testhelpers.sh +++ b/test/testhelpers.sh @@ -426,8 +426,14 @@ contains_same_elements() { exit $res } - is_stdin_attached() { test -t0 echo $? } + +has_test_dir() { + if [ -z "$GIT_LFS_TEST_DIR" ]; then + echo "No GIT_LFS_TEST_DIR. Skipping..." + exit 0 + fi +}