Bump xunit and related fixes

[sungam3r]

I suggest to say goodbye for netcoreapp3.1 and net5.0 since the-latest-and-greatest xunit does not support these TFMs, see #344 and #345.

[Shane32]

I suggest to say goodbye for netcoreapp3.1 and net5.0 since the-latest-and-greatest xunit does not support these TFMs, see #344 and #345.

I still use older TFMs in various applications and it does not hurt us to test for them. If you want to bump xunit, just make the dependency conditional on the tfm and use the older version for 3.1/5.0.

[Shane32]

This project doesn't target .NET 3.1 or .NET 5. So I'm fine with merging this PR as-is and dropping testing against those platforms.

[Shane32]

We should drop 3.1/5.0 from the GitHub Action scripts, however.

[Shane32]

Note that if we do drop .NET 3.1/5.0 testing, we implicitly drop .NET Standard 2.1 testing. 👎 Only the net6 TFM gets tested

[Shane32]

And it seems that we don't test against the .NET Framework currently (a supported TFM), which means that we are not testing the .NET Standard 2.0 build. 👎

[sungam3r]

True. Is it worth to?

[sungam3r]

We should drop 3.1/5.0 from the GitHub Action scripts, however.

Missed it. Removed. Regarding .NET Framework / netstandard2.0 tests - I'm 50/50.

[Shane32]

If we have have different builds for .NET Standard 2.0/2.1 we should test against it. It's not like it costs any dev time and CI testing is free. It would be nice if we could test each of the 3 builds against .NET 6, but that would likely be quite a bit more complicated than simply testing .NET 4.8, .NET 3.1 and .NET 6.0.

I certainly have GraphQL applications that run on .NET Framework under a fully supported scenario, as I expect others do...

[Shane32]

Arguably we could remove the .NET Standard 2.1 TFM from the codebase, using .NET Standard 2.0 as a fallback for older TFMs. I'm not prepared to do so at this time.

[sungam3r]

OK, I'll deal with this tomorrow making dependency conditional.

[Shane32]

Arguably we could remove the .NET Standard 2.1 TFM from the codebase, using .NET Standard 2.0 as a fallback for older TFMs. I'm not prepared to do so at this time.

I'll also review this. Perhaps there is little practical benefit to having a .NET Standard 2.1 TFM.

[Shane32]

Looking at the codebase, there isn't any code that wouldn't be exercised if we tested on both .NET Framework 4.8 and .NET 6, omitting .NET 3.1. Again, I'd probably just run it on 3.1 anyway, but if we do eliminate it, at least all the code paths get tested.

I wouldn't eliminate the .NET Standard 2.1 TFM. For one thing, it eliminates some unnecessary dependencies.

[Shane32]

We don't need to test against .NET 5. It isn't a LTM release, and .NET 3.1 testing will cover the .NET Standard 2.1 binary.

[sungam3r]

Hah.

Test run for /home/runner/work/parser/parser/src/GraphQLParser.Tests/bin/Debug/net462/GraphQLParser.Tests.dll (.NETFramework,Version=v4.6.2)
Microsoft (R) Test Execution Command Line Tool Version 17.7.1 (x64)
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:02.58] GraphQLParser.Tests: Catastrophic failure: System.TypeLoadException: Could not load type of field 'Xunit.Runner.VisualStudio.VsExecutionSink:recorder' (4) due to: Could not load file or assembly 'Microsoft.VisualStudio.TestPlatform.ObjectModel, Version=15.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies.

[sungam3r]

https://github.com/microsoft/vstest/issues/2469

[sungam3r]

9.2.1 ?

[Shane32]

No, 8.4.1 from master. You can merge master to develop and then release 9.2.1 from develop.

[Shane32]

Most people use the version referenced by GraphQL, which is still on v8. After release we should bump the parser dependency to 8.4.1 in GraphQL's master branch.

[Shane32]

Most all of the changes to 9.x I've backported to 8.x. The only real difference now is that 9.x has removed a bunch of constructors. Current versions of GraphQL.NET don't use those obsolete constructors anymore, so it is compatible with Parser 9, but maintains a dependency to 8.x so there are no binary breaking changes within 7.x (either by GraphQL.NET or GraphQL.NET Parser, its dependency).

[sungam3r]

I'm confused a bit. I just merged master into develop (actually fast-forwarded develop) and now master and develop both point to the same commit. What version to release - 8.4.1 or 9.2.1 ?

[sungam3r]

I'm done for today. You may go on and make release if you wish.

[Shane32]

Oh. Sorry, we are using the v8 branch for v8 and the master branch for v9

[Shane32]

(It's been that way since you released v9.) No matter. I'll cherry pick the commits to the v8 branch and issue releases.

[Shane32]

I changed the default branch to v8 so we don't make this mistake again.

[sungam3r]

Ok.