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

Disable format tests on Alpine #17247

Conversation

NikolaMilosavljevic
Copy link
Member

Format tests were enabled with #17238 That PR uncovered the issues with dotnet-format on Alpine that is tracked with dotnet/format#1945

This change allows skipping of dotnet-format tests on Alpine and disables them in Alpine CI leg.

@NikolaMilosavljevic NikolaMilosavljevic merged commit a37bcd1 into dotnet:release/8.0.1xx Aug 24, 2023
18 checks passed
@@ -19,7 +19,7 @@ public class DotNetFormatTests : SmokeTests
/// <Summary>
/// Format an unformatted project and verify that the output matches the pre-computed solution.
/// </Summary>
[Fact]
[SkippableFact(Config.ExcludeDotnetFormatEnv, skipOnTrue: true)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The disadvantage of this approach is that the test is going to fail by default when run on Alpine. This is going to impact our Alpine distro maintainer friends. An alternative approach is to utilize the Config.TargetRid and conditionally run the test based on if it contains "alpine". We do something similar for osx today - https://github.com/dotnet/dotnet/blob/main/test/Microsoft.DotNet.SourceBuild.SmokeTests/TestScenario.cs#L14

The other advantage of this approach is that I see this workaround as being temporary until the dotnet-format issue is resolved. Utilizing Config.TargetRid would be a more scoped change (only needing to touch one place in the code).

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