Small feature request: Use timeout provided for It() with done channel to modify slowSpecThreshold for that test only about ginkgo HOT 10 CLOSED

Makes sense about not modifying the slowSpecThreshold for tests known to be slow. It'll take a bit of work but should be doable (though I may not get to it soon)

As for skipping slow tests I tend to prefer using a tag (something in the describe/it like SLOW or INTEGRATION) and the use the built in -skip flag to filter out the slow tests. Do you think that's an OK workaround?

Onsi

Sent from my iPad

On Nov 2, 2013, at 2:16 PM, Ian Dawes [email protected] wrote:

If the timeout provided for an It() with a done channel is longer than the default 5 second timeout of slowSpecThreshold, it would be nice if we didn't have to modify the slowSpecThreshold for all tests to accommodate the one or two slow tests.

Also, it would be nice to have an option to skip known slow tests (i.e. the ones with a long timeout specified on the It())

—
Reply to this email directly or view it on GitHub.

from ginkgo.

Sounds good to me.

from ginkgo.

hey @idawes
I'm realizing I never got around to fixing this. do you still think it's necessary? I'm inclined to close the issue as I haven't really felt this to be a pain point.

Feel free to reopen if you think otherwise.

from ginkgo.

It's still a bit of a thorn in my side, because I have a bunch of tests that run long. Manually excluding them is pretty annoying.

If you don't think you'll get around to it ever, maybe I'll get around to a fix and a PR at some point.

from ginkgo.

is manually setting --slowSpecThreshold for those suites too onerous?

i'm in the middle of a major refactor and it might be much simpler to have
the timeout override the slowSpecThreshold.... will see.

onsi

On Mon, Mar 31, 2014 at 4:19 PM, Ian Dawes [email protected] wrote:

It's still a bit of a thorn in my side, because I have a bunch of tests
that run long. Manually excluding them is pretty annoying.

If you don't think you'll get around to it ever, maybe I'll get around to
a fix and a PR at some point.

Reply to this email directly or view it on GitHubhttps://github.com//issues/12#issuecomment-39154213
.

from ginkgo.

It's not fantastic, because it relaxes the timing requirements on all the
tests, not just the ones that are known to be long. It's not a pressing
issue at the moment, as I'm not dealing with big production test
environments yet.

On Mon, Mar 31, 2014 at 7:54 PM, Onsi Fakhouri [email protected]:

is manually setting --slowSpecThreshold for those suites too onerous?

i'm in the middle of a major refactor and it might be much simpler to have
the timeout override the slowSpecThreshold.... will see.

onsi

On Mon, Mar 31, 2014 at 4:19 PM, Ian Dawes [email protected]
wrote:

It's still a bit of a thorn in my side, because I have a bunch of tests
that run long. Manually excluding them is pretty annoying.

If you don't think you'll get around to it ever, maybe I'll get around to
a fix and a PR at some point.

Reply to this email directly or view it on GitHub<
https://github.com/onsi/ginkgo/issues/12#issuecomment-39154213>
.

Reply to this email directly or view it on GitHubhttps://github.com//issues/12#issuecomment-39156501
.

from ginkgo.

fair, I'll throw this into the 1.0 milestone.

from ginkgo.

Cool, thanks.
On Apr 1, 2014 4:00 PM, "Onsi Fakhouri" [email protected] wrote:

fair, I'll throw this into the 1.0 milestone.

Reply to this email directly or view it on GitHubhttps://github.com//issues/12#issuecomment-39251466
.

from ginkgo.

hey @idawes

I thought about it some more and I think that tying the slow-spec threshold to the async timeout isn't the right thing to do here.

first, because it's a timeout, you'll never see the test marked as slow -- it would only ever be marked as failed!

second, the timeout should - typically - be set to something a few factors higher than the expected time (to avoid flakiness on slower environments, for example). one might consider the test slow well below this threshold, though.

so i'm not sure. specifying a separate consider-me-slow time sounds onerous. and i agree that increasing the global timeout isn't great either, but i think it's closer to the truth: let me know when tests start to get slower than X so that I can go and fix them...?

from ginkgo.

I see your point. It occurs to me that the slow test threshold is somewhat
like a default for all tests, which could be locally overridden for
individual tests. The syntax is already there for specifying a timeout.
Perhaps it would be appropriate to re-imagine that value as a local slow
test threshold. Mostly that's the way I've been using it, setting it longer
than absolutely required to allow for variability in test time. Generally,
I'm perfectly happy to use the default value, and want to be notified if
any of the normal case tests take longer than that default and relatively
short threshold, but definitely don't want to be notified on every run that
includes the long tests.

On Sun, Apr 20, 2014 at 6:05 PM, Onsi Fakhouri [email protected]:

hey @idawes https://github.com/idawes

I thought about it some more and I think that tying the slow-spec
threshold to the async timeout isn't the right thing to do here.

first, because it's a timeout, you'll never see the test marked as slow --
it would only ever be marked as failed!

second, the timeout should - typically - be set to something a few factors
higher than the expected time (to avoid flakiness on slower environments,
for example). one might consider the test slow well below this threshold,
though.

so i'm not sure. specifying a separate consider-me-slow time sounds
onerous. and i agree that increasing the global timeout isn't great either,
but i think it's closer to the truth: let me know when tests start to get
slower than X so that I can go and fix them...?

Reply to this email directly or view it on GitHubhttps://github.com//issues/12#issuecomment-40906146
.

from ginkgo.