Added tests for unsupported braces in custom format specifiers.

[mikernet]

Test modifications for issue #35167 with related coreclr PR for the associated StringBuilder modifications here:

dotnet/coreclr#23062

[dnfclas]

All CLA requirements met.

[karelz]

@bartonjs can you please review the change?
@mikernet did you look at the test failures?

[mikernet]

@mikernet did you look at the test failures?

@karelz No I didn't, because it depends on the changes in the referenced CoreCLR pull request to succeed so you guys gotta do your inner loop testing process.

[mikernet]

@karelz I don't really know how the back and forth process works - the other PR was accepted, so how do I get it to rerun the tests against that?

[karelz]

inner loop testing = what anyone can run locally :)
Changes across CoreCLR and CoreFX are usually a bit more complicated and involved. We should wait for CoreCLR PR to be merged first. Then we can merge this one (marking it blocked).
That said, the dev is expected to run the tests on the CoreCLR changes locally prior to merging the PR in CoreCLR. Did you do that? (there are some advanced docs in our repo about it)

[mikernet]

Yes I ran them locally. The issue here is they are failing in CI.

[karelz]

@mikernet and they will be until CoreCLR PR is merged and ingested in CoreFX :)

[danmoseley]

I guess then it's waiting on #36480 (or its successor)

[ViktorHofer]

@mikernet seems you aren't blocked anymore, do you need any help to get this ready?

[ViktorHofer]

/azp run corefx-ci

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[mikernet]

@ViktorHofer I'm not really sure what you need me to do here. The tests compile and run successfully on my system with the changes in the other referenced PR.

[ViktorHofer]

The test is failing on OSX:

Assert.Equal() Failure\n ↓ (pos 0)\nExpected: 0}\nActual: }\n ↑ (pos 0)

   at System.Text.Tests.StringBuilderTests.AppendFormat_NoEscapedBracesInCustomFormatSpecifier() in /Users/vsts/agent/2.150.3/work/1/s/src/System.Runtime/tests/System/Text/StringBuilderTests.cs:line 933

[mikernet]

I don't know enough about how this build system works to figure out why that would be. If OSX is running the latest .NET Core string builder code that I modified in the referenced PR then there's no reason for it to fail - it's running the same code as the windows and linux builds. If the OSX test runs some other version of .NET / .NET Core then that test will need to be excluded from running on OSX.

There's an exclusion on that test for .NET Framework since that won't get the StringBuilder changes, so the test is marked with [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]. OSX should be running the latest .NET Core changes though should it not? Based on the test results it's fairly clear that it isn't though.

[mikernet]

Looking at the new results rolling in something odd is clearly going on and I don't really know why the new StringBuilder code doesn't seem to be running everywhere.

[mikernet]

Has the referenced PR even been merged yet? It is showing as open so I'm not sure what you want me to do.

[mikernet]

The stranger thing to me is that Linux musl_x64_Debug is passing but Linux musl_x64_Release is failing. One of them is apparently running skipping the [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)] test and the other isn't??

Why anything that isn't NETFX is passing I have no idea. That additional test should be failing anything not running the new StringBuilder code. This is all very confusing.

[JeremyKuhne]

Has the referenced PR even been merged yet?

I just finally got it merged yesterday. Hopefully the next CoreCLR integration will have the change- I'll keep an eye out for it.

[stephentoub]

Cherry-picked into #37953. Thanks.