Improve display name when DataRow contains arrays

[MichelZ]

Fixes #1333

Improves the display name for DataRow tests for Arrays from:
MyMethod (System.String[])
to
MyMethod ([a,b,c])

[Evangelink]

I wasn't able to review but just wanted to point out that this change would be a breaking change for people still using the display name for the tests id generation. Because it's obsolete it might not be as important but wanted to point it out so we don't merge too fast without thinking about consequences.

[MichelZ]

I wasn't able to review but just wanted to point out that this change would be a breaking change for people still using the display name for the tests id generation. Because it's obsolete it might not be as important but wanted to point it out so we don't merge too fast without thinking about consequences.

True. Do you think it would be helpful if the feature was an opt-in feature? I.e. a Flag on the Attribute for the Array Display Name?

[nohwnd]

@Evangelink Good to know I was thinking the same, but there was no breaking label, or future milestone. Can this be linked to id generation strategy? We have that as assembly level attribute (but imho also as a runsettings option). And if user is not using Legacy then we could generate the new display name?

https://grep.app/search?q=idgenerationstrategy&filter[repo.pattern][0]=testfx

[nohwnd]

The build is crashing on StackOverflow, I've added steps to install procdump so vstest can collect dumps. and pushed into your main Michel, for your next PR I would recommend creating a separate branch that is not main, so we don't block you if we take some time to merge this PR.

[nohwnd]

Correction I cannot push to your main, because that is not a feature branch. So I created this PR, once that is merged you should be able to update and see why is the code is failing with Stack Overflow. #3059

[MichelZ]

Thanks, that should not be necessary just for me though, I fixed the StackOverflow issue now

[nohwnd]

We will merge this next week at earliest, once evangelink and other folks are back from vacation and we can discuss how to avoid the breaking change.

If you are okay with this PR hanging here and blocking your main branch then no action is needed. If you want to dig into more stuff, I would suggest checking out another branch, and making a new PR from it.

[Evangelink]

We will expose a protected property on the DataRowAttribute that let subclasses know what's the selected TestIdGenerationStrategy and we will use this property to condition the change and avoid any breaking change.

@MichelZ I will push the change to your PR.

[Evangelink]

Thank you for your contribution @MichelZ

[Evangelink]

Flaky test, force merging.