Test hooks should not execute if all tests are skipped

[shakefu]

For example:

func TestSkipped(t *testing.T) {
    g := goblin.Goblin(t)
    g.Before(func () {
        panic("Tests are skipped, this shouldn't happen")
    })
    g.Xit(func (){ 
        g.Assert(false)
    })
}

This test suite should be a no-op, but will take action in the hooks (Before, After, etc.) even so.

This would be parity with the behavior when no tests are declared but there are hooks present.

My use case is using Before hooks to set up external services for the tests. We also allow a "fast" test mode when the services aren't present (by wrapping It and Xit from an environment flag).

Alternatively: Add a g.SkipIf(bool) to skip entire test suites if the passed value is true/false.

[shakefu]

This is the work-around we're currently using, a bit clumsy:

// Goblin is a partial interface for grabbing the funcs we need
type Goblin interface {
	It(string, ...interface{})
	Xit(string, ...interface{})
	Before(func())
	BeforeEach(func())
	JustBeforeEach(func())
	After(func())
	AfterEach(func())
	// Goblin funcs
	Assert(interface{}) *goblin.Assertion
	Fail(interface{})
	FailNow()
	Failf(string, ...interface{})
	Fatalf(string, ...interface{})
	Errorf(string, ...interface{})
	Helper()
}

type Skip struct {
	Goblin
}

func (this *Skip) It(name string, h ...interface{}) {
	this.Goblin.Xit(name, h...)
}
func (this *Skip) Before(f func()) {
}
func (this *Skip) BeforeEach(f func()) {
}
func (this *Skip) JustBeforeEach(f func()) {
}
func (this *Skip) After(f func()) {
}
func (this *Skip) AfterEach(f func()) {
}

// SkipIf returns a wrapped Goblin object if noSkip is false, otherwise it
// returns the normal Goblin instance passed in.
func SkipIf(g *goblin.G, noSkip bool) Goblin {
	if noSkip {
		return g
	}
	// Skip holds noop functions for skipping tests
	skip := &Skip{g}
	return skip
}

[marcosnils]

func TestSkipped(t *testing.T) {
    g := goblin.Goblin(t)
    g.Before(func () {
        panic("Tests are skipped, this shouldn't happen")
    })
    g.Xit(func (){ 
        g.Assert(false)
    })
}

This is not a valid example since it doesn't contain a Describe block

This would be parity with the behavior when no tests are declared but there are hooks present.

What do you mean with this? Parity with what exactly?

I think an alternative here would be to also provide a Xdescribe option that will skip all block altogether along with their hooks and tests. WDYT?

[shakefu]

This is not a valid example since it doesn't contain a Describe block
Bad example... hastily typed. Imagine there is one. xD

This would be parity with the behavior when no tests are declared but there are hooks present.
What do you mean with this? Parity with what exactly?

func MyTest(...){
    g := Goblin(...)
    g.Describe("no tests", func (){
        g.Before(func () {
            fmt.Println("This never runs because there are no tests.")
        })
    })
    g.Describe("one skipped test", func (){
        g.Before(func (){ 
            fmt.Println("This will run because there is a declared test, even tho it's empty and skipped")
        })
        g.Xit("placeholder test")
    })
}

... having gotten into the code more this is because notifyParents is always called in (g *G)It and (g *G)Xit... and hence hasTests will be set in the 2nd example (but not the first) and it'll run the hooks.

So this was unexpected behavior/non-parity between what I assumed were similar test suite states (e.g. no tests, or just skipped tests).

I think an alternative here would be to also provide a Xdescribe option that will skip all block altogether along with their hooks and tests. WDYT?

I've gone ahead and implemented an alternative in #106 that handles a variety of cases with tests... I'm using that successfully from my fork but would love to see it make it back upstream.

/cc @marcosnils