OpenCoverUI / OpenCover.UI

Visual Studio Integration to OpenCover
MIT License
77 stars 46 forks source link

Add basic XUnit Support #69

Closed joshlrogers closed 8 years ago

joshlrogers commented 9 years ago

Added basic support for XUnit Facts. Theories will be detected but the results will not be parsed and Trait/Category support is not yet functional. Had to adjust the nuget restore process to allow for XUnit test discovery to work through AppVeyor.

pver commented 9 years ago

Thanks for this @joshlrogers ! We have a small pending fix for the 0.8 release, this should be fixed in the coming days. After that I think we are good to merge this in for the 0.9 release.

joshlrogers commented 9 years ago

@pver thank you. You may want to consider sneaking in the fix I have in b72b70d for the 0.8 release as it was causing an extensions crash unrelated to my XUnit changes.

pver commented 9 years ago

@joshlrogers Thanks! :+1: I pulled your b72b70d commit in, the fix looks good. I cherry picked it from your branch, so that all credits go to you for this fix ;)

joshlrogers commented 9 years ago

@pver you may also want to consider fc57ede. This is another bug which causes the execute command to be unnecessarily disabled should a build fail on previous test execution. This requires an extension reset to be able to execute tests again.

pver commented 9 years ago

Thanks, I'll look into it. Are you sure the command wasn't enabled again after a (later) successful build? I saw that behavior as well, but in my case the command was available again after a successful (re)build (or at least after clicking the refresh button in the test explorer window). Did you test how the 'execute' behaves when there is no build result (yet)? I think @jamdagni86 wrote it on purpose in such a way that the tests/coverage can only be executed if the build was successful

joshlrogers commented 9 years ago

It doesn't become available again after subsequent builds, at least not in my testing it didn't. The problem being is that when the build fails and the event is called _isRunningCodeCoverage is still being set to true with the call to SetCommandAvailabilityStatus(false);

Then when a rebuild is performed and becomes successful OnTestRecoveryFinished is called but _isRunningCodeCoverage is still set to true from the previous build failure, it is never reset anywhere, so then Enabled was set to false because it believed the executor was still running.

I am at a loss to explain how it worked for you because I felt I had tested it quite thoroughly last night and I still am unable to see where _isCodeCoverageRunning is ever flipped back to true. Only point for it to be able to do that is through the finally clause in the RunOpenCover function but if you had a previous build failure you couldn't reach the point to actually execute RunOpenCover in the first place therefore being frozen until the extension was reset.

I also moved the management of the execution state into the executor itself so hopefully situations like this couldn't occur in the future.

pver commented 9 years ago

I'm also still getting used to the flow of commands in the extension architecture of Visual Studio, we should find a way to make it more clear in the code what happens when. I'll do some further testing with it, but your approach looks good. We also plan to make things more (unit) testable in the future so all improvements are more than welcome!