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

Restore parallel tests execution #403

Merged
merged 3 commits into from
Nov 15, 2023

Conversation

0xced
Copy link
Member

@0xced 0xced commented Nov 15, 2023

This reduces the tests execution time in half. On my machine: 76 seconds when run in parallel vs 144 seconds when run serially.

The fix introduced in 41bc33d (running dotnet test on the csproj rather then on the built dlls) was problematic for two reasons.

  1. It rebuilds Serilog.Settings.Configuration.Tests.csproj in Debug instead of Release and it's unnecessary since it was previously built.
  2. It doesn't run tests in parallel anymore, increasing the total tests execution time.

The right fix was to disable deterministic source paths for the test project with <DeterministicSourcePaths>false</DeterministicSourcePaths>. This way, the [CallerFilePath] attribute works as expected with absolute paths on the current machine instead of being substituted with /_/ (which was causing the test failures). This looks like a breaking change that was introduced in the .NET 8 SDK.

Also, I added back allowPrerelease = false and rollForward = latestFeature in the global.json file.

So that the [CallerFilePath] attribute works as expected with absolute paths on the current machine instead of being substituted with /_/
….json

Was removed in 725b6b2 but will be useful when the next versions of the .NET 8 SDK will be published.
@nblumhardt
Copy link
Member

Awesome! How on earth did you find that? 🎉

@nblumhardt nblumhardt merged commit 1d0e7bf into serilog:dev Nov 15, 2023
1 check passed
@0xced 0xced deleted the restore-parallel-tests branch November 16, 2023 22:28
@0xced
Copy link
Member Author

0xced commented Nov 16, 2023

Well, I've become kind of an MSBuild expert in spite of myself over the years. The /_/ path was a great hint towards SourceLink, so I looked at SourceLink related stuff with MSBuild Binary and Structured Log Viewer (thanks again @KirillOsenkov 🙏) and eventually discovered this property.

Also, I was pretty motivated because I already had spend way too much figuring out how to run test in parallel across target frameworks. 😄

@KirillOsenkov
Copy link

Yes, .NET 8 enabled SourceLink by default. Happy the tool is helpful!

@nblumhardt nblumhardt mentioned this pull request Jun 6, 2024
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