Roslyn: Code fix tests should accept title of code fix instead of the index

[YairHalberstadt]

I'm currently adding an extra option to the various implement Interface code fixes - implement remaining members of interface explicitly. See https://github.com/dotnet/roslyn/issues/41558.

There already are 6 different code fixes which are offered by this CodeFixProvider. All of them are offered under different circumstances. The current method for testing a specific one is to supply the index of the code fix out of all those offered. Of course this is highly brittle, and you can easily end up testing the wrong code fix.

Testing that a specific code fix isn't offered is even more tricky. The best you can do is count the number of code fixes offered, but this doesn't tell you which wasn't offered.

@CyrusNajmabadi suggests testing every single code fix. This still makes it tricky to make sure that the code fix you think you're testing is actually the one you intended to test. It also remains brittle to new code fixes being added.

I would suggest adding an extra option to allow supplying the title of the code fix instead of the index to the test. Testing that the code fix isn't offered now becomes much simpler as well, though it remains brittle in a different way - changing the title of the code fix now breaks the validity of the test.

I would be interested in hearing ideas for better designs...

[YairHalberstadt]

cc @jnm2

[jnm2]

This has bothered me for a while too.

[CyrusNajmabadi]

This test addition can just be made in your pr for the feature :-)

[CyrusNajmabadi]

It also remains brittle to new code fixes being added.

NOte: this is a virtue for me. If i change a feature such that it would have new/different fixes for a user, then i would like to know what scenarios change by having existing tests start failing. It gives something very relevant to look at to know if the feature change is working properly.

[sharwell]

This was already fixed in the testing library (Microsoft.CodeAnalysis.Testing: https://github.com/dotnet/roslyn-sdk/issues/171). All of Roslyn's analyzer and code fix tests should be converted to the new library for improved functional coverage and accuracy.

[YairHalberstadt]

@sharwell
That's good to hear!

Could you link to an example of it being used please? Thanks.

[sharwell]

https://github.com/dotnet/roslyn-analyzers/blob/79c2638f54b9e7ef0a3954cc838ec4c13ea63c1f/src/NetAnalyzers/UnitTests/Microsoft.CodeQuality.Analyzers/QualityGuidelines/SealMethodsThatSatisfyPrivateInterfacesTests.Fixer.cs#L160-L161

[sharwell]

If you specify just one of the available properties (index or key), it will use the one specified. If you specify both, it will additionally verify that the fix with the given key appears at the given index.