Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow tests to be run serially rather than in parallel #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rafrombrc
Copy link

No description provided.

@ghthor
Copy link

ghthor commented Nov 15, 2012

I'm curious, what is your use case that requires spec's to run serially?

Another way to implement this would be Runner turning into an interface with ConcurrentRunner and SerialRunner types implementing the Runner interface. Also, should specify the behavior SerialRunner's behavior.

@rafrombrc
Copy link
Author

On Wed 14 Nov 2012 09:13:33 PM PST, W. Walthall wrote:

I'm curious, what is your use case that requires spec's to run serially?

We're using go-notify (for now, might change later) to have a simple
event system in our app. The app spins up various long-running
goroutines, and then listens for STOP events to perform clean-up at
shutdown time.

In my tests I'm exercising these pieces independently, and I need to
send the STOP event to each component to tear things down when the test
has completed. If several tests are running in parallel, these STOP
events will be picked up by all of the listeners, wreaking havoc since
the other test setups will start tearing down prematurely.

Beyond that, have tests run in parallel by default just makes me
nervous when testing code that is also very parallel. I'm not the only
one who feels this way, it's the reason that Go's test runner doesn't
run tests in parallel. In this thread you can see Rob Pike explaining
that he won't accept a parallel option for Go's test runner unless
tests have a way to explicitly say "I won't work in parallel, don't run
me in parallel" that is honored by the test runner:
https://groups.google.com/forum/?fromgroups=#!topic/golang-dev/RAKiqi44GEU

Another way to implement this would be |Runner| turning into an
interface with |ConcurrentRunner| and |SerialRunner| types
implementing the |Runner| interface. Also, should specify the behavior
|SerialRunner|'s behavior.

Yeah, I know there are better ways to do this. It definitely smells a
bit weird to still be using the whole scheduledTask infrastructure in
the runner when we know that we're always running the tests serially,
adding all of the scheduled tasks and then removing them one by one.
But after losing a couple of days wrestling w/ my tests (not
exclusively due to this issue, only complicated by it) I only had the
bandwidth to make the smallest effective fix so I could get back to
some forward movement on my actual application. It's fine w/ me if you
don't want to merge what I've done. If that's the case, I'll flag a
better, tested fix as you've described as something to try to get to,
but I can't guarantee it'll happen soon.

Thanks for your reply! And thanks for gospec in the first place... I've
had to make a couple of tweaks (in particular I've had to stuff a
trivial testing.T implementation in so I can use gomock, see
https://github.com/mozilla-services/heka/blob/dev/pipeline/support_test.go#L22),
but overall it's been very nice to spell our tests out in a nice BDD
spec manner.

Cheers,

-r


Reply to this email directly or view it on GitHub
#6 (comment).

@ghthor
Copy link

ghthor commented Nov 15, 2012

I'm not the creator of gospec, I'm just a user as yourself. Thanks for the read from the mailing list and I respect your use case. The only reason I asked was I was legitimately curious(as in, "I wonder if I'm going to run into problems?") as I've been using gospec extensively and had yet to run into any issues.

I understand having limited time to produce a more adequate solution. I've done the same when adding support for ignored specs. I'll see what I can do using my idea for the Runner interface.

Cheers,
-w

@rafrombrc
Copy link
Author

On Thu 15 Nov 2012 01:03:13 PM PST, W. Walthall wrote:

I'm not the creator of gospec, I'm just a user as yourself.

Ah, whoops, sorry. :)

Thanks for
the read from the mailing list and I respect your use case. The only
reason I asked was I was legitimately curious(as in, "I wonder if I'm
going to run into problems?") as I've been using gospec extensively
and had yet to run into any issues.

Right, no problem. Yeah, everything will be fine as long as you don't
rely on any global state that tests running in parallel might step on.

I understand having limited time to produce a more adequate solution.
I've done the same when adding support for ignored specs. I'll see
what I can do using my idea for the |Runner| interface.

Yup, it doesn't seem like it would be too hard at all. I'll also keep
it flagged on my mental "to-do" list. If you do work out a better
patch, let me know, I can maybe merge it back to my own copy of the
repo. :)

-r

Cheers,
-w


Reply to this email directly or view it on GitHub
#6 (comment).

@ghthor
Copy link

ghthor commented Nov 15, 2012

I will certainly share it with you. Keep on trucking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants