paulegradie / Sailfish

Sailfish - a production friendly performance benchmark runner for .NET
https://paulgradie.com/Sailfish/
MIT License
11 stars 4 forks source link

Global setup exception does not contain the TestCaseId #167

Closed shaun-od closed 1 month ago

shaun-od commented 1 month ago

Describe the bug When an exception is thrown in the lifecycle method SailfishGlobalSetup the exception appears in the CompiledTestCaseResult but without a TestCaseId.

To Reproduce Steps to reproduce the behavior:

  1. Run the existing Sailfish Test GlobalSetupExceptionsAreHandled
  2. Check the result object returned result.ExecutionSummaries[0].CompiledTestCaseResults[0].TestCaseId is null

Expected behavior I would expect the test invoked when the setup method is being called that throws the exception, would be included in the returned object.

shaun-od commented 1 month ago

Looking at the code, I think this is because the method CatchAndReturn in the SailfishExecutionEngine creates and returns a TestCaseExecutionResult using the constructor that takes just the exception.

Couldn't it create a new TestCaseExecutionResult using the preexisting constructor that allows the TestInstanceContainer and Exception to be passed in?

paulegradie commented 1 month ago

Thanks @shaun-od I'll address this today. Let me know if you're planning on putting up a PR 🙏

paulegradie commented 1 month ago

using the preexisting constructor that allows the TestInstanceContainer and Exception to be passed in

Yes - this is the way 👍