mrpmorris / Fluxor

Fluxor is a zero boilerplate Flux/Redux library for Microsoft .NET and Blazor.
MIT License
1.22k stars 139 forks source link

DisposableCallback InvalidOperationException when running unit tests with XPlat Code Coverage #448

Closed Riccorbypro closed 5 months ago

Riccorbypro commented 10 months ago

I am receiving the following error when my Azure Pipelines runner attempts to run my unit tests (bUnit/xUnit) with Fluxor.

2023-08-30T07:56:31.6242077Z Starting test execution, please wait...
2023-08-30T07:56:31.6629284Z A total of 1 test files matched the specified pattern.
2023-08-30T07:56:38.4925725Z The active test run was aborted. Reason: Test host process crashed : Unhandled exception. System.InvalidOperationException: DisposableCallback with Id ""StateSubscriber.Subscribe / SigniFlow.WebApp.Client.Shared.User.UserSignature" (created in "C:\Data\Mine\Code\Fluxor\Source\Lib\Fluxor\StateSubscriber.cs" on line 50)" was not disposed. See https://github.com/mrpmorris/Fluxor/tree/master/Docs/disposable-callback-not-disposed.md for more details
2023-08-30T07:56:38.4926802Z    at Fluxor.DisposableCallback.Finalize() in C:\Data\Mine\Code\Fluxor\Source\Lib\Fluxor\DisposableCallback.cs:line 75

In the above log, the component that caused the error was the UserSignature component, but I've seen various components from our project be the "culprit" here.

Running through the suggestions in disposable-callback-not-disposed.md have not helped to resolve this issue.

Upon further investigation, I noticed something weird. If I ran the tests locally, they all ran fine. Same thing from within both IDEs I use (JetBrains Rider and VS2022). Code coverage within those two also worked fine.

However, we use SonarQube for our code quality analysis, and this requires that we add --collect:"XPlat Code Coverage" to our dotnet test command to output compatible code coverage files using coverlet. The moment I tried to run my local SonarQube scanner, the error appeared again. This seems to be the cause of the error. If I remove the coverlet collection command, the error disappears.

Below are various dotnet test command combinations that I've tested and their results:

Command: dotnet test .\SigniFlow.sln --logger trx --results-directory Z:\TempResults\ --collect:"XPlat Code Coverage" Result: Failure

The active test run was aborted. Reason: Test host process crashed : Unhandled exception. System.InvalidOperationException: DisposableCallback with Id ""StateSubscriber.Subscribe / SigniFlow.WebApp.Client.Shared.User.ProfilePicture.ProfilePicture" (created in "C:\Data\Mine\Code\Fluxor\Source\Lib\Fluxor\StateSubscriber.cs" on line 50)" was not disposed. See https://github.com/mrpmorris/Fluxor/tree/master/Docs/disposable-callback-not-disposed.md for more details
   at Fluxor.DisposableCallback.Finalize() in C:\Data\Mine\Code\Fluxor\Source\Lib\Fluxor\DisposableCallback.cs:line 75

Command: dotnet test .\SigniFlow.sln --logger trx --results-directory Z:\TempResults\ Result: Pass

Command: dotnet test .\SigniFlow.sln --collect:"XPlat Code Coverage" Result: Failure

The active test run was aborted. Reason: Test host process crashed : Unhandled exception. System.InvalidOperationException: DisposableCallback with Id ""StateSubscriber.Subscribe / SigniFlow.WebApp.Client.Pages.DocPrepper.ActionBar.PrepperAction" (created in "C:\Data\Mine\Code\Fluxor\Source\Lib\Fluxor\StateSubscriber.cs" on line 50)" was not disposed. See https://github.com/mrpmorris/Fluxor/tree/master/Docs/disposable-callback-not-disposed.md for more details
   at Fluxor.DisposableCallback.Finalize() in C:\Data\Mine\Code\Fluxor\Source\Lib\Fluxor\DisposableCallback.cs:line 75

Is there any workaround for this issue? I don't see anyone else reporting this error manifesting in this manner.

Riccorbypro commented 10 months ago

So interestingly, even removing all the coverage-related commands other than "publish results" from the Azure pipeline still causes it to error out. It might not be related to the collector so much as something on Azure Pipelines' side. The coverage command still helps reproduce it locally though.

Azure Pipelines command: dotnet test SigniFlow.sln --logger trx --results-directory /home/vsts/work/_temp Result Failure

2023-08-30T11:14:10.1107436Z The active test run was aborted. Reason: Test host process crashed : Unhandled exception. System.InvalidOperationException: DisposableCallback with Id ""StateSubscriber.Subscribe / SigniFlow.WebApp.Client.Pages.DocPrepper.ActionBar.PrepperAction" (created in "C:\Data\Mine\Code\Fluxor\Source\Lib\Fluxor\StateSubscriber.cs" on line 50)" was not disposed. See https://github.com/mrpmorris/Fluxor/tree/master/Docs/disposable-callback-not-disposed.md for more details
2023-08-30T11:14:10.1108124Z    at Fluxor.DisposableCallback.Finalize() in C:\Data\Mine\Code\Fluxor\Source\Lib\Fluxor\DisposableCallback.cs:line 75
Riccorbypro commented 8 months ago

Just to update this - we've temporarily solved this by just not throwing the error if the running environment is DEBUG. This works for now since the release build is still capable of throwing this exception, but our unit tests all run successfully and are able to produce coverage reports.

I don't believe this is a solution more than a workaround though - I'd hope a better solution can be figured out.

mrpmorris commented 8 months ago

Is this page relevant?

https://bunit.dev/docs/interaction/dispose-components.html

Riccorbypro commented 5 months ago

Is this page relevant?

https://bunit.dev/docs/interaction/dispose-components.html

Not directly, but combing through the bUnit docs did alert me to the fact that we were running a somewhat non-standard test setup (capturing the TestContext rather than inheriting from it). After a bit of a refactor of our testing projects to closer align with bUnit's docs, I found a pattern in which tests failed which eventually led to a component that had overridden Dispose() without calling base.Dispose(). This, as per the docs, was our culprit.

The offending component was usually a child of the component which appeared in the error thrown by Fluxor. Hence why it seemed so random as to which component Fluxor thought was the culprit. xUnit tends to run tests in an unpredictable order, so often the first test that would run into issues wasn't consistent.

This component was also created way back at the start of the project, before the various libraries and systems we'd be using had been finalised. Obviously the code included in the first commit of the repo didn't go through review, so none of us caught it. The unit test project is basically the same story - by the time most of us were brought on the projects were all created and basic utility classes (such as a base test class that performs all the mocking and setup for "always needed" services that other Blazor tests could inherit from) had already been written.

While my issue is therefore solved, I think there may be an improvement to be made here. Having read through the implementations of DisposableCallback and StateSubscriber, I don't think it would be worth it to attempt to find the exact component that's caused the error and return that, as due to the nature of the error Fluxor wouldn't know that it caused the failure anyways. Perhaps a note in the documentation linked in the error message warning that this error may be thrown due to a child of the offending component not disposing properly, therefore prompting to not only check that IDisposable is correctly implemented on the main component but all of its children as well.

Apologies for the delay in response by the way - I had put this on our housekeeping list, but we hadn't had a chance to revisit it until recently.