feat(server): add more tests for the SSH server

arschles commented 8 years ago

This PR expands test coverage, mainly to the ./pkg/sshd package. Additionally, it pulls out the cleaner locking logic to a withCleanerLock func in /pkg/sshd/server.go

Highlights of the added/improved tests:

A test for concurrent pushes, each to a separate repository
Improving the previously-named TestPush test to attempt a larger number of concurrent pushes to the same repo. This test is currently skipped (with t.SkipNow()) and should be enabled after #209 is merged
Improvements to speed up tests in server_test.go by removing the sleep in the SSH handler func

arschles commented 8 years ago

Current output of go test -race ./pkg/sshd/...:

==================
WARNING: DATA RACE
Write by goroutine 115:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes.func1()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:181 +0xa3

Previous write by goroutine 114:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes.func1()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:181 +0xa3

Goroutine 115 (running) created at:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:186 +0xc59
  testing.tRunner()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:456 +0xdc

Goroutine 114 (running) created at:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:186 +0xc59
  testing.tRunner()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:456 +0xdc
==================
[info] 2016/02/29 17:02:21 Channel type: session
==================
WARNING: DATA RACE
Write by goroutine 115:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes.func1()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:181 +0xd1

Previous write by goroutine 114:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes.func1()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:181 +0xd1

Goroutine 115 (running) created at:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:186 +0xc59
  testing.tRunner()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:456 +0xdc

Goroutine 114 (running) created at:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:186 +0xc59
  testing.tRunner()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:456 +0xdc
==================
[info] 2016/02/29 17:02:21 Channel type: session
==================
WARNING: DATA RACE
Read by goroutine 115:
  github.com/deis/builder/vendor/golang.org/x/crypto/ssh.(*Session).Output()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/vendor/golang.org/x/crypto/ssh/session.go:297 +0x71
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes.func1()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:183 +0x1fd

Previous write by goroutine 116:
  github.com/deis/builder/vendor/golang.org/x/crypto/ssh.newSession()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/vendor/golang.org/x/crypto/ssh/session.go:554 +0x5c
  github.com/deis/builder/vendor/golang.org/x/crypto/ssh.(*Client).NewSession()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/vendor/golang.org/x/crypto/ssh/client.go:133 +0x117
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes.func1()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:181 +0x6e

Goroutine 115 (running) created at:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:186 +0xc59
  testing.tRunner()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:456 +0xdc

Goroutine 116 (running) created at:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:186 +0xc59
  testing.tRunner()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:456 +0xdc
==================
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
==================
WARNING: DATA RACE
Write by goroutine 118:
  testing.(*common).FailNow()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:335 +0x41
  testing.(*common).Fatalf()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:379 +0x94
  testing.(*T).Fatalf()
      <autogenerated>:33 +0x81
  github.com/deis/builder/vendor/github.com/arschles/assert.NoErr()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/vendor/github.com/arschles/assert/assert.go:104 +0x169
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes.func1()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:184 +0x288

Previous write by goroutine 115:
  testing.(*common).FailNow()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:335 +0x41
  testing.(*common).Fatalf()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:379 +0x94
  testing.(*T).Fatalf()
      <autogenerated>:33 +0x81
  github.com/deis/builder/vendor/github.com/arschles/assert.NoErr()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/vendor/github.com/arschles/assert/assert.go:104 +0x169
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes.func1()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:184 +0x288

Goroutine 118 (running) created at:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:186 +0xc59
  testing.tRunner()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:456 +0xdc

Goroutine 115 (finished) created at:
  github.com/deis/builder/pkg/sshd.TestManyConcurrentPushes()
      /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:186 +0xc59
  testing.tRunner()
      /private/var/folders/q8/bf_4b1ts2zj0l7b0p1dv36lr0000gp/T/workdir/go/src/testing/testing.go:456 +0xdc
==================
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
[info] 2016/02/29 17:02:21 Channel type: session
--- FAIL: TestManyConcurrentPushes (5.27s)
    <autogenerated>:33: /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:184: expected no error but got ssh: Stdout already set
    <autogenerated>:33: /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:184: expected no error but got ssh: Stdout already set
    <autogenerated>:33: /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:184: expected no error but got ssh: Stdout already set
[info] 2016/02/29 17:02:26 Listening on 127.0.0.1:2246
[info] 2016/02/29 17:02:26 Accepting new connections.
[info] 2016/02/29 17:02:26 Checking closer.
[info] 2016/02/29 17:02:26 Accepted connection.
[info] 2016/02/29 17:02:26 Checking closer.
[info] 2016/02/29 17:02:26 Channel type: session
[info] 2016/02/29 17:02:26 Channel type: session
FAIL
FAIL    github.com/deis/builder/pkg/sshd    9.246s

arschles commented 8 years ago

The previous race has been fixed in c65d33f. Current status of tests is an expected failure, due to the global lock:

--- FAIL: TestManyConcurrentPushes (1.27s)
    <autogenerated>:33: /Users/aaronschlesinger/gocode/src/github.com/deis/builder/pkg/sshd/server_test.go:186: expected no error but got waitgroup wait timed out

arschles commented 8 years ago

I've pushed https://github.com/arschles/deis-builder/commit/f4b5870db86e329ce8e37aa96d6a83fb3b65a35d, which skips the TestManyConcurrentPushes test. This test is blocked by the global push lock. cc/ @smothiki

bacongobbler commented 8 years ago

pushlock reads much cleaner now. Nice implementation :+1:

deis / builder

feat(server): add more tests for the SSH server #212