Disallow execution of private fixtures (breaks stack trace printing)

[jerrdasur]

https://github.com/smartystreets/gunit/issues/15

[jerrdasur]

@mdwhatcott got it! It's my first PR for open source. So, please, don't be very angry if I missed something important :)

[codecov-io]

Codecov Report

Merging #26 into master will increase coverage by 0.23%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #26      +/-   ##
==========================================
+ Coverage   89.08%   89.31%   +0.23%     
==========================================
  Files           6        6              
  Lines         229      234       +5     
==========================================
+ Hits          204      209       +5     
  Misses         18       18              
  Partials        7        7

Impacted Files	Coverage Δ
runner.go	86.36% <100%> (+4.01%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1f2b2fa...2f9d8a2. Read the comment docs.

[mdwhatcott]

@jerrdasur - Thanks very much, this is looking great! I'll pull your changes and poke around a bit. I'll be in contact with you shortly.

[mdwhatcott]

@jerrdasur - I've just done a few tests on the master branch and have found that a fixture type need not be exported. It's the methods of the type that must be exported. Could you go into more detail about the error you were seeing?

[jerrdasur]

@mdwhatcott I've double checked my example from issue https://github.com/smartystreets/gunit/issues/15#issuecomment-545419017 As you can see, all methods of the structure are exportable. But the structure itself isn't. So, methods must be exportable and so they are. But private fixture can't provide information about path for correct stack_trace.

go test -v ./profile
=== RUN   TestManagerFixture
=== PAUSE TestManagerFixture
=== CONT  TestManagerFixture
=== RUN   TestManagerFixture/TestBaseExample
=== PAUSE TestManagerFixture/TestBaseExample
=== RUN   TestManagerFixture/TestUnknownUser
=== PAUSE TestManagerFixture/TestUnknownUser
=== CONT  TestManagerFixture/TestBaseExample
=== CONT  TestManagerFixture/TestUnknownUser
--- FAIL: TestManagerFixture (0.00s)
    --- PASS: TestManagerFixture/TestBaseExample (0.00s)
        test_case.go:67: Test definition:

    --- FAIL: TestManagerFixture/TestUnknownUser (0.00s)
        test_case.go:67: Test definition:

        fixture.go:100: 
            Expected: '(*errors.errorString){s:"unknown user"}'
            Actual:   'nil'
            (Should resemble)!

FAIL
FAIL    jerrfm-app/profile      0.197s
FAIL

Output with exportable structure:

go test -v ./profile
=== RUN   TestManagerFixture
=== PAUSE TestManagerFixture
=== CONT  TestManagerFixture
=== RUN   TestManagerFixture/TestBaseExample
=== PAUSE TestManagerFixture/TestBaseExample
=== RUN   TestManagerFixture/TestUnknownUser
=== PAUSE TestManagerFixture/TestUnknownUser
=== CONT  TestManagerFixture/TestBaseExample
=== CONT  TestManagerFixture/TestUnknownUser
--- FAIL: TestManagerFixture (0.00s)
    --- PASS: TestManagerFixture/TestBaseExample (0.00s)
        test_case.go:67: Test definition:
            /Users/jerrdasur/go/src/jerrfm-app/profile/manager_test.go:25
    --- FAIL: TestManagerFixture/TestUnknownUser (0.00s)
        test_case.go:67: Test definition:
            /Users/jerrdasur/go/src/jerrfm-app/profile/manager_test.go:29
        fixture.go:100: 
            Expected: '(*errors.errorString){s:"unknown user"}'
            Actual:   'nil'
            (Should resemble)!

FAIL
FAIL    jerrfm-app/profile      0.328s
FAIL

[jerrdasur]

@mdwhatcott I've prepared simple example example/basic_test.go:

package example

import (
    "testing"

    "github.com/smartystreets/assertions/should"
    "github.com/smartystreets/gunit"
)

func TestBasicFixture(t *testing.T) {
    gunit.Run(new(basicFixture), t)
}

type basicFixture struct {
    *gunit.Fixture // Required: Embedding this type is what makes the magic happen.
}

func (f *basicFixture) TestBaseExample() {
    f.So(1, should.BeNil)
}

Out:

go test -v ./example
=== RUN   TestBasicFixture
=== PAUSE TestBasicFixture
=== CONT  TestBasicFixture
=== RUN   TestBasicFixture/TestBaseExample
=== PAUSE TestBasicFixture/TestBaseExample
=== CONT  TestBasicFixture/TestBaseExample
--- FAIL: TestBasicFixture (0.00s)
    --- FAIL: TestBasicFixture/TestBaseExample (0.00s)
        test_case.go:67: Test definition:

        fixture.go:100: 
            Expected: nil
            Actual:   '1'

FAIL
FAIL    jerrfm-app/example      0.328s
FAIL

Output with exportable structure:

go test -v ./example
=== RUN   TestBasicFixture
=== PAUSE TestBasicFixture
=== CONT  TestBasicFixture
=== RUN   TestBasicFixture/TestBaseExample
=== PAUSE TestBasicFixture/TestBaseExample
=== CONT  TestBasicFixture/TestBaseExample
--- FAIL: TestBasicFixture (0.00s)
    --- FAIL: TestBasicFixture/TestBaseExample (0.00s)
        test_case.go:67: Test definition:
            /Users/jerrdasur/go/src/jerrfm-app/example/basic_test.go:18
        fixture.go:100: 
            Expected: nil
            Actual:   '1'

FAIL
FAIL    jerrfm-app/example      0.334s
FAIL

[mdwhatcott]

@jerrdasur - I had forgotten that this is about the printing of stack traces (file:line), not about execution. Thanks for that reminder! Ok, I'll continue my review of your changes.

[mdwhatcott]

@jerrdasur - Things are looking pretty good. I think there is a problem with the way you are attempting to detect whether the test fixture is exported. I'm seeing the warning message appear for any fixture, even those that are exported. Here's a snippet of the output that I'm seeing:

mike@Mike-MBP:~/src/github.com/smartystreets/gunit [jerrdasur-feature/warning-private-fixtures]
• go test -v
=== RUN   TestFinalizeAfterNoActions
--- PASS: TestFinalizeAfterNoActions (0.00s)
=== RUN   TestFinalizeAfterFailure
--- PASS: TestFinalizeAfterFailure (0.00s)
...
    runner.go:50: Type (*gunit.FixtureWithNoTestCases) is not exportable
...
    runner.go:50: Type (*gunit.RunnerFixtureSetupTeardown) is not exportable
...
    runner.go:50: Type (*gunit.RunnerFixturePlain) is not exportable
...
    runner.go:50: Type (*gunit.RunnerFixtureFocus) is not exportable
...
    runner.go:50: Type (*gunit.RunnerFixtureFocusLong) is not exportable
...
    runner.go:50: Type (*gunit.RunnerFixtureWithOnlyOneFocus) is not exportable
...
    runner.go:50: Type (*gunit.privateFixture) is not exportable
    --- PASS: TestWarningOfPrivateFixture/Test1 (0.00s)
        test_case.go:67: Test definition:

PASS
ok      github.com/smartystreets/gunit  0.010s

Maybe something like this would work more reliably?

func isExportedType(fixtureType reflect.Type) bool {
    name := fixtureType.String()
    parts := strings.Split(name, ".")
    last := parts[len(parts)-1]
    return unicode.IsUpper(rune(last[0]))
}

for a type name like *gunit.privateFixture I'm just splitting the string on the dot (.) and looking at the first character of the type name to see if it is uppercase.