search

serilog / serilog-settings-configuration

A Serilog configuration provider that reads from Microsoft.Extensions.Configuration
Apache License 2.0
446 stars 129 forks source link

Fix build script and move approval test into new project #372

Closed sungam3r closed 1 year ago

sungam3r commented 1 year ago

See https://github.com/serilog/serilog/pull/1887 . I moved approval test into new project to align build process with core repo.

SimonCropp commented 1 year ago

why split out approval test?

sungam3r commented 1 year ago

To align build process with core repo where approval test is its own project.

SimonCropp commented 1 year ago

i would argue the core project should have i merged back into the main test project @nblumhardt

0xced commented 1 year ago

If merged, this will break my efforts to run tests in parallel on AppVeyor which works around https://github.com/dotnet/sdk/issues/19147.

sungam3r commented 1 year ago

i would argue the core project should have i merged back into the main test project

I'm fine either way.

nblumhardt commented 1 year ago

I've just added approval tests to Serilog.Sinks.OpenTelemetry directly in the main unit test project. Needing to rely on an additional assertions library (Shouldly) is a marginal negative but probably not enough to justify the additional project, since we're already very relaxed about test project dependencies.

Seems like this one should be closed RE the parallel testing effort; if it needs more consideration or reopening please just sing out :)