Add SpecContext to ReportAfterSuite callback body.

[eugenenosenko]

No description provided.

[onsi]

hey sorry for the delay - will take a look at this and share feedback.

[onsi]

hey - this is good but I'd like to add a test that exercises passing the context into ReportAfterSuite. Would you be up for adding a test to https://github.com/onsi/ginkgo/blob/master/internal/internal_integration/report_suite_test.go - it doens't have to be super-exhaustive but at least one test that covers:

• passing a SpecContext to ReportAfterSuite
• correct behavior when a timeout occurs (i.e. a timeout failure, a configurable timeout, etc.)

this should be additive - I want to keep coverage of the non-context variant a well.

Let me know if you're up for that - the internal integration tests can take a bit to wrap your head around but you should be able to copy and paste and pattern-match your way to victory.

[eugenenosenko]

sure

[eugenenosenko]

@onsi sorry was a bit busy 😄 updated the PR and also docs

[onsi]

thjanks! one last thing - rather than add ReportAfterSuiteContext I'd prefer to simply change ReportAfterSuite to accept any body (or to use generics to accept one of the two allowed types. Are you up for that change? If not I can do it after I merge this in.

[eugenenosenko]

@onsi Yeah I was squimish about adding XyzContext api since it didn't fit with existing functions and initially I wanted to have a generic param i.e. :

// could be reused for other functions that do reporting
type ReporterBody interface {
	func(Report) | func(SpecContext, Report)
}

func ReportAfterSuite[B ReporterBody](text string, body B, args ...interface{}) bool {
	combinedArgs := []interface{}{body}
	combinedArgs = append(combinedArgs, args...)
	return pushNode(internal.NewNode(deprecationTracker, types.NodeTypeReportAfterSuite, text, combinedArgs...))
}

but this caused another issue here since generic function has to be instantiated

so DSL code would have to have twe aliased functions:

var ReportAfterSuite2 = ginkgo.ReportAfterSuite[func(ginkgo.SpecContext, ginkgo.Report)]
var ReportAfterSuite = ginkgo.ReportAfterSuite[func(ginkgo.Report)]

which might be valid approach but also could introduce some confusion ( but let me know your opinion )

or alternatively to use any as you've mentioned but that would in turn reduce clarity of the existing API. I'm fine with making those changes but let me know what is your preferred approach

[onsi]

hey @eugenenosenko - thanks for giving the generics approach a try. let’s go with any. It’s how much of Ginkgo operates anyway so it’s in keeping with the established pattern.

Thanks for working on this! I appreciate the contribution :)

[eugenenosenko]

@onsi my pleasure 😄 I've updated the PR and I think that with little effort similar changes can be made to the ReportBeforeSuite and after reporting hooks

[onsi]

wonderful! this looks great, thank you :) if you're up for it since you've done all this work already converting the other nodes would be great (either in this PR or a subsequent one would be fine by me).

thanks so much!

[eugenenosenko]

updated the code to include changes to other reporting nodes 😄

[eugenenosenko]

@onsi are you ok with the proposed changes? 😄

[onsi]

hey @eugenenosenko yes - thanks for the nudge. i was waiting for CI to run and then totally forgot to check back! this all looks good to me. i’ll pull it in and cut a release soon.

[eugenenosenko]

@onsi great, if you have any other improvement tasks let me know, we use ginkgo extensively so I'll be glad to contribute

[onsi]

thanks @eugenenosenko ! this just shipped in v2.16.0