Skip to content
This repository has been archived by the owner on May 29, 2018. It is now read-only.

Move tests to standard library #3

Merged
merged 6 commits into from
Feb 23, 2015

Conversation

uovobw
Copy link

@uovobw uovobw commented Feb 20, 2015

As per object, remove gocheck.v1 from the project and only use the stdlib testing package.

@@ -29,12 +24,11 @@ func (c *fakeClock) since(t time.Time) time.Duration {
return c.elapsed
}

var _ = Suite(&S{})

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why just have a single TestAll? If you use separate exported TestXxx funcs (instead of testXxx), then you can use -test.run=pattern to selectively run certain tests, and you get more informative failure messages.

And you can just factor the common setup code in the top of the TestAll func to a helper func like func setup() { ... }.

@uovobw
Copy link
Author

uovobw commented Feb 23, 2015

new version with requested changes. i went with the TestAll solution since the way it was written forced a new contributor to notice the setup() and tearDownTest() methods being necessary for each test call, but your solution works as well and gives more feedback on failures.

sqs added a commit that referenced this pull request Feb 23, 2015
@sqs sqs merged commit e2fdd7d into sourcegraph:master Feb 23, 2015
uovobw added a commit to uovobw/httpcache that referenced this pull request Feb 24, 2015
cache := New(testServer)
if cache == recover() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equivalent to if cache == nil {.

From the Go spec:

The return value of recover is nil if any of the following conditions holds:

  • panic's argument was nil;
  • the goroutine is not panicking;
  • recover was not called directly by a deferred function.

recover is not being called directly by a deferred function here, so its return value will always be nil. It can never be called when a panic is taking place, since it's not deferred, and it will never be executed if a panic happens before it runs.

dmitshur added a commit that referenced this pull request Mar 29, 2016
The motivation for using idiomatic style and standard library for tests
is that it's more universal, and more familiar to a larger set of people.
It makes it easier to read, modify tests for experienced Go programmers.
It does not require thorough knowledge and familiarity with custom APIs.

Resolves #41.

This commit is largely based on previous work by @uovobw in
#3, modified by me to
improve error messages, style, and address preliminary review comments.

more
dmitshur added a commit that referenced this pull request Mar 29, 2016
The motivation for using idiomatic style and standard library for tests
is that it's more universal, and more familiar to a larger set of people.
It makes it easier to read, modify tests for experienced Go programmers.
It does not require thorough knowledge and familiarity with custom APIs.

Resolves #41.

This commit is largely based on previous work by @uovobw in
#3, modified by me to
improve error messages, style, and address preliminary review comments.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants