Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup test project #45

Merged
merged 2 commits into from
Feb 10, 2021
Merged

Cleanup test project #45

merged 2 commits into from
Feb 10, 2021

Conversation

0xced
Copy link
Member

@0xced 0xced commented Feb 3, 2021

  • Update .NET Core target from .NET Core 1.0 to .NET Core 3.1 (LTS)
  • Use latest versions of Microsoft.NET.Test.Sdk, xunit.runner.visualstudio and xunit
  • Remove everything unneeded to build and run tests

Running tests now work in both IDEs and with dotnet test command line invocation.

* Update .NET Core target from .NET Core 1.0 to .NET Core 3.1 (LTS)
* Use latest versions of Microsoft.NET.Test.Sdk, xunit.runner.visualstudio and xunit
* Remove everything unneeded to build and run tests
* Update Visual Studio image from 2017 to 2019 on AppVeyor

Running tests now work in both IDEs and with `dotnet test` command line invocation.
@0xced 0xced force-pushed the cleanup_test_project branch from 9efa8b0 to 16c81f8 Compare February 3, 2021 17:42
@0xced
Copy link
Member Author

0xced commented Feb 3, 2021

Oh, it's a duplicate of #40. But with the latest versions of the test dependencies... And it goes a little further by deleting unused file and even harmful files. The launchSettings.json file was causing JetBrains Rider to not be able to run the tests at all.

@nblumhardt
Copy link
Member

Thanks for the PR! Would you mind if I merge #40 and we apply your additional changes on top? :-)

@0xced
Copy link
Member Author

0xced commented Feb 3, 2021

That's fine.

@Numpsy
Copy link
Member

Numpsy commented Feb 4, 2021

I missed those uneeded netcoreapp1.0 conditions when I made my other PR, but I thought it useful to build the test project as both netcore 2.1 and 3.1 because the main Serilog project still seems to do both? (that's a call for $management$ though)

@Numpsy
Copy link
Member

Numpsy commented Feb 7, 2021

saying that, is it reasonable to run the tests against .net5 as well at this point?

@nblumhardt
Copy link
Member

I think so 👍 - maybe we queue that up next.

RE coverage, the core Serilog lib is very conservative and tests on just about everything, but in sink/enricher projects we tend to keep it to one or two framework configurations in order to make things more manageable. I wouldn't be against adding some more targets, but the two here seem pretty reasonable :-)

@nblumhardt nblumhardt merged commit e605df2 into serilog:dev Feb 10, 2021
@0xced 0xced deleted the cleanup_test_project branch February 10, 2021 22:34
@nblumhardt nblumhardt mentioned this pull request Aug 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants