Merge pull request #6858 from hashicorp/tests_race_detector

test with race detection enabled + fix race on progressbar

.travis.yml

@@ -1,5 +1,10 @@
 env:
-    - USER=travis
+  - USER=travis
+
+os:
+  - windows
+  - linux
+  - osx
 
 sudo: false
 
@@ -9,11 +14,15 @@ go:
   - 1.11.x
   - master
 
-install:
-  - make deps
+before_install:
+  - >
+    if [[ "$TRAVIS_OS_NAME" == "windows" ]];
+    then choco install -y make;
+    fi
 
 script:
-  - GOMAXPROCS=2 make ci
+  - df -h
+  - make ci
 
 branches:
   only:
@@ -22,4 +31,5 @@ branches:
 matrix:
   allow_failures:
     - go: master
+    - os: windows
   fast_finish: true

Makefile

@@ -11,7 +11,7 @@ GOPATH=$(shell go env GOPATH)
 # gofmt
 UNFORMATTED_FILES=$(shell find . -not -path "./vendor/*" -name "*.go" | xargs gofmt -s -l)
 
-EXECUTABLE_FILES=$(shell find . -type f -perm +111 | egrep -v '^\./(website/vendor|vendor/|\.git|bin/|scripts/|pkg/)' | egrep -v '.*(\.sh|\.bats|\.git)' | egrep -v './provisioner/ansible/test-fixtures/exit1')
+EXECUTABLE_FILES=$(shell find . -type f -executable | egrep -v '^\./(website/[vendor|tmp]|vendor/|\.git|bin/|scripts/|pkg/)' | egrep -v '.*(\.sh|\.bats|\.git)' | egrep -v './provisioner/ansible/test-fixtures/exit1')
 
 # Get the git commit
 GIT_DIRTY=$(shell test -n "`git status --porcelain`" && echo "+CHANGES" || true)
@@ -21,9 +21,11 @@ GOLDFLAGS=-X $(GIT_IMPORT).GitCommit=$(GIT_COMMIT)$(GIT_DIRTY)
 
 export GOLDFLAGS
 
-default: deps generate test dev
+.PHONY: bin checkversion ci default deps fmt fmt-docs fmt-examples generate releasebin test testacc testrace updatedeps
 
-ci: deps test
+default: deps generate testrace dev releasebin package dev fmt fmt-check mode-check fmt-docs fmt-examples 
+
+ci: testrace
 
 release: deps test releasebin package ## Build a release build
 
@@ -49,7 +51,6 @@ deps:
 	@go get golang.org/x/tools/cmd/stringer
 	@go get -u github.com/mna/pigeon
 	@go get github.com/kardianos/govendor
-	@go get golang.org/x/tools/cmd/goimports
 
 dev: deps ## Build and install a development build
 	@grep 'const VersionPrerelease = ""' version/version.go > /dev/null ; if [ $$? -eq 0 ]; then \
@@ -100,25 +101,25 @@ generate: deps ## Generate dynamically generated code
 	goimports -w common/bootcommand/boot_command.go
 	gofmt -w command/plugin.go
 
-test: deps fmt-check mode-check ## Run unit tests
+test: fmt-check mode-check vet ## Run unit tests
 	@go test $(TEST) $(TESTARGS) -timeout=2m
-	@go tool vet $(VET)  ; if [ $$? -eq 1 ]; then \
-		echo "ERROR: Vet found problems in the code."; \
-		exit 1; \
-	fi
 
 # testacc runs acceptance tests
 testacc: deps generate ## Run acceptance tests
 	@echo "WARN: Acceptance tests will take a long time to run and may cost money. Ctrl-C if you want to cancel."
 	PACKER_ACC=1 go test -v $(TEST) $(TESTARGS) -timeout=45m
 
-testrace: deps ## Test for race conditions
+testrace: fmt-check mode-check vet ## Test with race detection enabled
 	@go test -race $(TEST) $(TESTARGS) -timeout=2m
 
 updatedeps:
 	@echo "INFO: Packer deps are managed by govendor. See .github/CONTRIBUTING.md"
 
+vet: ## Vet Go code
+	@go tool vet $(VET)  ; if [ $$? -eq 1 ]; then \
+		echo "ERROR: Vet found problems in the code."; \
+		exit 1; \
+	fi
+
 help:
 	@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
-
-.PHONY: bin checkversion ci default deps fmt fmt-docs fmt-examples generate releasebin test testacc testrace updatedeps

builder/hyperv/iso/builder_test.go

@@ -1,3 +1,5 @@
+// +build !windows
+
 package iso
 
 import (

builder/hyperv/vmcx/builder_test.go

@@ -21,6 +21,7 @@ func testConfig() map[string]interface{} {
 		"iso_url":                 "http://www.packer.io",
 		"shutdown_command":        "yes",
 		"ssh_username":            "foo",
+		"switch_name":             "switch", // to avoid using builder.detectSwitchName which can lock down in travis-ci
 		"ram_size":                64,
 		"guest_additions_mode":    "none",
 		"clone_from_vmcx_path":    "generated",

builder/parallels/common/output_config_test.go

@@ -1,3 +1,5 @@
+// +build !windows
+
 package common
 
 import (

builder/virtualbox/common/step_shutdown_test.go

@@ -133,9 +133,9 @@ func TestStepShutdown_shutdownDelay(t *testing.T) {
 	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
 		t.Fatalf("bad action: %#v", action)
 	}
-	testDuration := time.Since(start).Seconds()
-	if testDuration < 2.5 || testDuration > 2.6 {
-		t.Fatal("incorrect duration")
+	testDuration := time.Since(start)
+	if testDuration < 2500*time.Millisecond || testDuration > 2700*time.Millisecond {
+		t.Fatalf("incorrect duration %s", testDuration)
 	}
 
 	if _, ok := state.GetOk("error"); ok {
@@ -158,9 +158,9 @@ func TestStepShutdown_shutdownDelay(t *testing.T) {
 	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
 		t.Fatalf("bad action: %#v", action)
 	}
-	testDuration = time.Since(start).Seconds()
-	if testDuration > 0.6 {
-		t.Fatal("incorrect duration")
+	testDuration = time.Since(start)
+	if testDuration > 600*time.Millisecond {
+		t.Fatalf("incorrect duration %s", testDuration)
 	}
 
 	if _, ok := state.GetOk("error"); ok {

common/iso_config_test.go

@@ -1,3 +1,5 @@
+// +build !windows
+
 package common
 
 import (

provisioner/ansible/provisioner_test.go

@@ -1,3 +1,5 @@
+// +build !windows
+
 package ansible
 
 import (

template/parse_test.go

@@ -1,3 +1,5 @@
+// +build !windows
+
 package template
 
 import (

vendor/github.com/cheggaaa/pb/pb.go

@@ -13,7 +13,7 @@ import (
 )
 
 // Current version
-const Version = "1.0.25"
+const Version = "1.0.26"
 
 const (
 	// Default refresh rate - 200ms
@@ -159,12 +159,16 @@ func (pb *ProgressBar) Add64(add int64) int64 {
 
 // Set prefix string
 func (pb *ProgressBar) Prefix(prefix string) *ProgressBar {
+	pb.mu.Lock()
+	defer pb.mu.Unlock()
 	pb.prefix = prefix
 	return pb
 }
 
 // Set postfix string
 func (pb *ProgressBar) Postfix(postfix string) *ProgressBar {
+	pb.mu.Lock()
+	defer pb.mu.Unlock()
 	pb.postfix = postfix
 	return pb
 }
@@ -273,6 +277,8 @@ func (pb *ProgressBar) NewProxyReader(r io.Reader) *Reader {
 }
 
 func (pb *ProgressBar) write(total, current int64) {
+	pb.mu.Lock()
+	defer pb.mu.Unlock()
 	width := pb.GetWidth()
 
 	var percentBox, countersBox, timeLeftBox, timeSpentBox, speedBox, barBox, end, out string
@@ -300,12 +306,10 @@ func (pb *ProgressBar) write(total, current int64) {
 	}
 
 	// time left
-	pb.mu.Lock()
 	currentFromStart := current - pb.startValue
 	fromStart := time.Now().Sub(pb.startTime)
 	lastChangeTime := pb.changeTime
 	fromChange := lastChangeTime.Sub(pb.startTime)
-	pb.mu.Unlock()
 
 	if pb.ShowElapsedTime {
 		timeSpentBox = fmt.Sprintf(" %s ", (fromStart/time.Second)*time.Second)
@@ -394,13 +398,12 @@ func (pb *ProgressBar) write(total, current int64) {
 
 	// check len
 	out = pb.prefix + timeSpentBox + countersBox + barBox + percentBox + speedBox + timeLeftBox + pb.postfix
+
 	if cl := escapeAwareRuneCountInString(out); cl < width {
 		end = strings.Repeat(" ", width-cl)
 	}
 
 	// and print!
-	pb.mu.Lock()
-	defer pb.mu.Unlock()
 	pb.lastPrint = out + end
 	isFinish := pb.isFinish
 

vendor/github.com/cheggaaa/pb/pb_x.go

@@ -20,6 +20,7 @@ var (
 	echoLockMutex    sync.Mutex
 	origTermStatePtr *unix.Termios
 	tty              *os.File
+	istty            bool
 )
 
 func init() {
@@ -28,13 +29,18 @@ func init() {
 
 	var err error
 	tty, err = os.Open("/dev/tty")
+	istty = true
 	if err != nil {
 		tty = os.Stdin
+		istty = false
 	}
 }
 
 // terminalWidth returns width of the terminal.
 func terminalWidth() (int, error) {
+	if !istty {
+		return 0, errors.New("Not Supported")
+	}
 	echoLockMutex.Lock()
 	defer echoLockMutex.Unlock()
 
@@ -51,26 +57,28 @@ func terminalWidth() (int, error) {
 func lockEcho() (shutdownCh chan struct{}, err error) {
 	echoLockMutex.Lock()
 	defer echoLockMutex.Unlock()
-	if origTermStatePtr != nil {
-		return shutdownCh, ErrPoolWasStarted
+	if istty {
+		if origTermStatePtr != nil {
+			return shutdownCh, ErrPoolWasStarted
+		}
+
+		fd := int(tty.Fd())
+
+		origTermStatePtr, err = unix.IoctlGetTermios(fd, ioctlReadTermios)
+		if err != nil {
+			return nil, fmt.Errorf("Can't get terminal settings: %v", err)
+		}
+
+		oldTermios := *origTermStatePtr
+		newTermios := oldTermios
+		newTermios.Lflag &^= syscall.ECHO
+		newTermios.Lflag |= syscall.ICANON | syscall.ISIG
+		newTermios.Iflag |= syscall.ICRNL
+		if err := unix.IoctlSetTermios(fd, ioctlWriteTermios, &newTermios); err != nil {
+			return nil, fmt.Errorf("Can't set terminal settings: %v", err)
+		}
+
 	}
-
-	fd := int(tty.Fd())
-
-	origTermStatePtr, err = unix.IoctlGetTermios(fd, ioctlReadTermios)
-	if err != nil {
-		return nil, fmt.Errorf("Can't get terminal settings: %v", err)
-	}
-
-	oldTermios := *origTermStatePtr
-	newTermios := oldTermios
-	newTermios.Lflag &^= syscall.ECHO
-	newTermios.Lflag |= syscall.ICANON | syscall.ISIG
-	newTermios.Iflag |= syscall.ICRNL
-	if err := unix.IoctlSetTermios(fd, ioctlWriteTermios, &newTermios); err != nil {
-		return nil, fmt.Errorf("Can't set terminal settings: %v", err)
-	}
-
 	shutdownCh = make(chan struct{})
 	go catchTerminate(shutdownCh)
 	return
@@ -79,16 +87,18 @@ func lockEcho() (shutdownCh chan struct{}, err error) {
 func unlockEcho() error {
 	echoLockMutex.Lock()
 	defer echoLockMutex.Unlock()
-	if origTermStatePtr == nil {
-		return nil
+	if istty {
+		if origTermStatePtr == nil {
+			return nil
+		}
+
+		fd := int(tty.Fd())
+
+		if err := unix.IoctlSetTermios(fd, ioctlWriteTermios, origTermStatePtr); err != nil {
+			return fmt.Errorf("Can't set terminal settings: %v", err)
+		}
+
 	}
-
-	fd := int(tty.Fd())
-
-	if err := unix.IoctlSetTermios(fd, ioctlWriteTermios, origTermStatePtr); err != nil {
-		return fmt.Errorf("Can't set terminal settings: %v", err)
-	}
-
 	origTermStatePtr = nil
 
 	return nil

vendor/github.com/cheggaaa/pb/pool.go

@@ -90,7 +90,9 @@ func (p *Pool) writer() {
 // Restore terminal state and close pool
 func (p *Pool) Stop() error {
 	p.finishOnce.Do(func() {
-		close(p.shutdownCh)
+		if p.shutdownCh != nil {
+			close(p.shutdownCh)
+		}
 	})
 
 	// Wait for the worker to complete

vendor/vendor.json

@@ -657,12 +657,12 @@
 			"revision": "50da7d4131a3b5c9d063932461cab4d1fafb20b0"
 		},
 		{
-			"checksumSHA1": "7GMQgpfoSQv4QcaREE2GzSZvlQI=",
+			"checksumSHA1": "UBznrWrDuttNZf56nubBl2nG4Ws=",
 			"path": "github.com/cheggaaa/pb",
-			"revision": "2af8bbdea9e99e83b3ac400d8f6b6d1b8cbbf338",
-			"revisionTime": "2018-05-21T09:56:06Z",
-			"version": "v1.0.25",
-			"versionExact": "v1.0.25"
+			"revision": "007b75a044e968336a69a6c0c617251ab62ac14c",
+			"revisionTime": "2018-10-16T15:56:12Z",
+			"version": "v1.0.26",
+			"versionExact": "v1.0.26"
 		},
 		{
 			"checksumSHA1": "X2/71FBrn4pA3WcA620ySVO0uHU=",