search

reportportal / agent-net-specflow

Report Portal agent for SpecFlow
Apache License 2.0
10 stars 4 forks source link

Fix handling of BeforeFeature #84

Closed maaex closed 1 month ago

maaex commented 4 months ago

This fixes #75

Currently, when exceptions occur in BeforeFeature and BeforeTestRun, they are swallowed by SafeBindingInvoker and are not displayed anywhere to the user. The tests continue to run as usual (see passing ThisScenarioShouldFailBecauseOfFeatureFailsBefore and all passing tests when uncommenting the throw statement in the BeforeTestRun hook in ReportPortal.SpecFlowPlugin.IntegrationTests.Hooks.

This PR makes it so failures in BeforeFeature and BeforeTestRun causes test failures. In addition, any exception thrown in BeforeFeature can be seen in the error message in RP for all scenarios in that feature. Exceptions in BeforeTestRun will cause all scenarios to have the "Skipped" status.

nvborisenko commented 4 months ago

Thank you @maaex , this is major issue we wanted to fix.

Is it possible to fail scenarios if BeforeTestRun hook fails?

maaex commented 4 months ago

Thank you @maaex , this is major issue we wanted to fix.

Is it possible to fail scenarios if BeforeTestRun hook fails?

There is no FeatureContext available at that point so the solution would have to be different. I noticed that there is a TestThreadContext with a TestError property. I could probably use that in the same way that I'm using FeatureContext for BeforeFeature, but I have no idea what other effects (if any) that would have. What do you think?

Edit: Looking at the SpecFlow Docs, maybe TestThreadContext really is what I should use.

nvborisenko commented 4 months ago

In general applying SafeBindingInvoker approach is kind of workaround, we can do everything here what we need... despite the fact that user may register his own SafeBindingInvoker. We don't care.

The right solution would be https://github.com/reportportal/agent-net-specflow/issues/49 but it is not ready yet, at least not ready for us.

maaex commented 4 months ago

There seems to be some weird race conditions going on with the TestThreadContext. When I'm handling it in the same way as the FeatureContext in my PR, the tests pass when running them normally but fail when I debug them.

If possible, I would prefer if we could merge this PR first and look into improvements later?

nvborisenko commented 1 month ago

@maaex Sorry for long absence, your fix is released as v3.6.2. Thank you for contribution.

maaex commented 1 month ago

@nvborisenko thank you! No worries, we have been using a forked version, I'm just happy that we can update now :)