installer icon indicating copy to clipboard operation
installer copied to clipboard

Update the fluent assertions version to improve test failure output.

Open marcpopMSFT opened this issue 1 year ago • 7 comments

Include a failing tests to verify if the output is better.

marcpopMSFT avatar Dec 15 '23 00:12 marcpopMSFT

@vitek-karas I was looking at https://github.com/dotnet/runtime/commit/faa272f860f83802a9cee65619e6e2e841b05433 which improved the output for the tests in AzDo. That was later removed when fluent assertions was updated but making the same change here was not sufficient. Do you know why your workaround was no longer needed in runtime?

marcpopMSFT avatar Dec 15 '23 17:12 marcpopMSFT

Also, AssertionScope doesn't appear to have a definition for AddFailure. I'm trying to figure out why it did in the runtime when your change went in but doesn't here.

marcpopMSFT avatar Dec 15 '23 17:12 marcpopMSFT

Updating the version of Fluent didn't work and I still can't get the AddFailure workaround that Vitek had put into place to work in this repo. @vitek-karas how did you end up being able to remove your workaround and why doesn't it work for C:\repos\installer\test\Microsoft.DotNet.Tools.Tests.Utilities\Assertions\AssertionScopeExtensions.cs(14,32): error CS1061: 'AssertionScope' does not contain a definition for 'AddFailure' and no access ible extension method 'AddFailure' accepting a first argument of type 'AssertionScope' could be found (are you missing a using directive or an assembly reference?) [C:\repos\installer\test\Microsoft.Do tNet.Tools.Tests.Utilities\Microsoft.DotNet.Tools.Tests.Utilities.csproj]

marcpopMSFT avatar Jan 02 '24 23:01 marcpopMSFT

The FluentAssertions library changed. The version I was working against back then was 4.19.4, you're working with 6.12.0. Looking in disassembler, it seems the method just got renamed though. Now it's AssertionScope.AddPreFormattedFailure - its implementation is identical to the old AssertionScope.AddFailure.

vitek-karas avatar Jan 04 '24 12:01 vitek-karas

@vitek-karas thanks for the pointer. I missed that originally as there were two spots we have the fluent version listed and I hadn't updated the one the tests actually use.

The issue I'm having now is that the assertionScope.Succeeded was moved to be internal so I can't check when to call AddPreFormattedFailure. The FailWith method has access to that property and it uses it to determine whether to record the failure. There's a HasFailures method but it checks if there is already a failure (which there isn't at the time of the call).

I was not able to find any good examples of how to detect the failure and call AddPreFormattedFailure at that time and the new version of Fluent still has the new line issues. Any other ideas?

marcpopMSFT avatar Jan 12 '24 23:01 marcpopMSFT

I looked into this a bit and the original FluentAssertion issue is indeed fixed but now you're running into the xunit console runner issue where it still escapes newlines in the xml result file in the <message>: https://github.com/dotnet/runtime/issues/1664 image

This was worked around for runtime by unescaping the xml as part of the helix reporter: https://github.com/dotnet/arcade/pull/7323

The installer repo doesn't use Helix so you're still hitting this, there is a workaround though: switch to the VSTest runner which produces .trx output files.

I've pushed a change to do that, here's what the the trx and html files look now:

image image

akoeplinger avatar Apr 22 '24 12:04 akoeplinger

It works in AzDO: image

akoeplinger avatar Apr 22 '24 13:04 akoeplinger

Thanks for the help here @akoeplinger In the SDK repo we're going to try to just get our installer tests running in helix so this change isn't needed there but with 8.0.1xx around for another couple of years, I think this is useful to have once the branch opens back up.

marcpopMSFT avatar Jun 20 '24 20:06 marcpopMSFT