reportportal / agent-net-specflow

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

Added support of custom HTTP handler in SpecFlow agent #32

Closed abasau closed 6 years ago

abasau commented 6 years ago

@nvborisenko I mostly agree with you that extension via code is more preferable than extension via configuration. But it's going to be more difficult here than it seems.

In order to inject custom HTTP Message Handler, it has to be done in ReportPortalAddin's static constructor. Our usual way to extend is to add an event. But in this case the attempt to subscribe to the event (ReportPortalAddin.Initializing += ReportPortalAddin_Initializing;) will trigger static constructor and the subscription will happen after the event is fired.

As a possible solution, we could move Service initialization from ReportPortalAddin's static constructor to ReportPortalHooks.BeforeTestRun but it's going to be a significant behavior change that may have unexpected consequences.

What do you think?

Regarding adding this feature to other clients. I usually try to refrain from introducing changes if they were not requested. I wouldn't want to spend time writing something just in case and then maintaining it.

nvborisenko commented 6 years ago

My opinion we can easily remove static initialization, why not. And initialize the plugin later, providing a way for considering custom initialization (in [BeforeTestRun] hook?).

abasau commented 6 years ago

I will move Service initialization to BeforeTestRun hook. Works for me. The change will break backward compatibility only if somebody uses Bridge.Service before the launch is started.

abasau commented 6 years ago

@nvborisenko The changes are ready to be reviewed again.

nvborisenko commented 6 years ago

It seems plugin should not provide context objects to subscriber, subscriber is able to get it via context injection. Added example: https://github.com/reportportal/example-net-specflow/blob/master/src/Example.SpecFlow/Hooks/Hooks1.cs#L15