jstemmer / go-junit-report

Convert Go test output to JUnit XML
MIT License
776 stars 224 forks source link

Subtests that fail report their parent tests as separate test cases #72

Closed venantius closed 2 years ago

venantius commented 6 years ago

This might be unavoidable, but it's a little annoying -

We maintain a Google App Engine app with a test suite that relies on an app engine shared dev server. We initialize the dev server in one test and defer its shutdown while running all other tests as subtests, e.g.:

func TestAuthenticationHandlers(t *testing.T) {
    var err error
    testInstance, err = aetest.NewInstance(nil)
    if err != nil {
        t.Fatalf("Failed to create instance: %v", err)
    }
    defer testInstance.Close()

    t.Run("AuthenticationHandlerTests", func(t *testing.T) {
        t.Run("LoginHandler", testLoginHandler)
        t.Run("RegisterHandler", testRegisterHandler)
        t.Run("VerifyEmailHandler", testVerifyEmailHandler)
        t.Run("ResetPasswordHandler", testResetPasswordHandler)
        t.Run("SetPasswordHandler", testSetPasswordHandler)
        t.Run("TestChangePasswordHandler", testChangePasswordHandler)
        t.Run("TestTOTPChangeSeedHandler", testTOTPChangeSeedHandler)
        t.Run("TestTOTPSetupHandler", testTOTPSetupHandler)
        t.Run("TestTOTPVerificationHandler", testTOTPVerificationHandler)
        t.Run("TestTOTPConfirmChangeHandler", testTOTPConfirmChangeHandler)
        t.Run("TestTOTPConfirmSetupHandler", testTOTPConfirmSetupHandler)
    })
}

If one of those subtests fails (let's say LoginHandler), go-junit-report describes 3 separate tests as having failed: TestAuthenticationHandlers, TestAuthenticationHandlers/AuthenticationHandlerTests and TestAuthenticationHandlers/AuthenticationHandlerTests/LoginHandler

I'm not sure if there's a way to only report the lowest-level test failing, but it'd certainly be useful.

The only obvious solution I can think of to our problem is to move away from this model of running tests and just use TestMain, but I thought I'd bring it up anyways.

jstemmer commented 6 years ago

go-junit-report doesn't really know the difference between regular tests and subtests, so every step is reported as its own test. It's not that obvious how these should be treated, since it's also possible to call t.Errorf within one of the intermediate tests. In that case you'd probably want to report that as a separate failure.

However, I do agree that it might be a little misleading to report 3 failures when in fact only a single test has failed. I might do something about this in the future, so I'll leave this issue open for now.

venantius commented 6 years ago

If someone wanted to "fix" this - the subtests are reported with indentation, e.g.:

--- PASS: TestSetPasswordHandler (0.01s)
    --- PASS: TestSetPasswordHandler/AMissingEmailCantSetPassword (0.00s)
    --- PASS: TestSetPasswordHandler/AMissingCodeCantSetPassword (0.00s)
    --- PASS: TestSetPasswordHandler/AValidPayloadCanSetPassword (0.26s)
    --- PASS: TestSetPasswordHandler/AnExpiredCodeCantSetPassword (0.13s)
    --- PASS: TestSetPasswordHandler/AMissingPasswordCantSetPassword (0.00s)
    --- PASS: TestSetPasswordHandler/AnInvalidPasswordCantSetPassword (0.00s)
    --- PASS: TestSetPasswordHandler/AnInvalidEmailCantSetPassword (0.00s)

So the simplest way of addressing it would be to drop the parent test and report anything indented thereafter.

But there are valid reasons for reporting it the way it sits - as currently reported, for instance, we log time spent running test setup / teardown. So if this was something that someone wanted to address, we'd probably want to put it behind a CLI flag.

jstemmer commented 2 years ago

This has been fixed in v2.0.0 (via 1b7027fde7d21ff395bb4e18ee371c30bf174fde and 9ad16898a8044f83800984add0907d4e1c070ecb). Use the -subtest-mode flag to either ignore subtest parent failures (-subtest-mode=ignore-parent-results) or drop the parent completely in the generated report (-subtest-mode=exclude-parents).