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

[Feature request] Tests with supported platform attributes #2820

Open
sliekens opened this issue Nov 12, 2023 · 6 comments
Open

[Feature request] Tests with supported platform attributes #2820

sliekens opened this issue Nov 12, 2023 · 6 comments

Comments

@sliekens
Copy link

Hi,

.NET 5 added a new platform compatibility analyzer, and along with it, new attributes to use in library code:

I'm using this for example to annotate code that only works on Windows. I have tests for this code, but right now I don't really have a good way to skip these tests when running in Linux.

image

It would be super cool if you could annotate a Fact with a platform compatibility attribute, such that the runner will dynamically skip it on unsupported platforms.

image

Thanks for your consideration.

@bradwilson
Copy link
Member

bradwilson commented Nov 14, 2023

In v3, we have added Assert.Skip and friends, which can do this at runtime. An example from our tests:

[Fact]
public async ValueTask CanDiscoverTests()
{
Assert.SkipWhen(EnvironmentHelper.IsMono, "Mono does not fully support dynamic assemblies");

We do not have Assert.Skip in v2.


We are unlikely to provide this as a standalone attribute, even in the context of v3, for the same reason that we rejected the common [ExpectedException] from NUnit/MSTest.

We do have an open discussion issue on whether we should add the ability to do SkipWhen or SkipUnless in the [Fact] attribute, which would need to point to a static field/property to get the runtime decision on skipping or not.

@bradwilson
Copy link
Member

Skimming through the documentation for this attribute, this doesn't actually feel like a runtime request, and I don't think you need us to do anything here.

Instead, it feels to me like what you should be doing is multi-targeting your test projects with a generic TFMs (like net5.0) as well as any OS-specific TFMs (like net5.0-linux, net5.0-windows, etc.), and then using blocks around Windows-specific tests like #if WINDOWS.

I'll admit that the documentation I've found here so far is terrible when trying to determine what the actual platforms are and what pre-processor directives might exist for those platforms by default, so my suggestion here may be wrong, but I think it should set you in the right direction.

@bradwilson
Copy link
Member

bradwilson commented Nov 14, 2023

Yeah, this seems to be functional right now.

Project:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFrameworks>net6.0;net6.0-windows</TargetFrameworks>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.7.2" />
    <PackageReference Include="xunit" Version="2.6.1" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.5.3" />
  </ItemGroup>

</Project>

Unit test:

using System.Runtime.Versioning;
using Xunit;

namespace Empty;

public static class MyMath
{
	[SupportedOSPlatform("windows")]
	public static int Add(int x, int y) => x + y;

	public static int Subtract(int x, int y) => x - y;
}

public class UnitTest
{
#if WINDOWS
	[Fact]
	public void TestAdd()
	{
		Assert.Equal(5, MyMath.Add(3, 2));
	}
#endif

	[Fact]
	public void TestSubtract()
	{
		Assert.Equal(1, MyMath.Subtract(3, 2));
	}
}

Running dotnet test on Windows:

  Determining projects to restore...
  All projects are up-to-date for restore.
  empty -> C:\Dev\repro\empty\bin\Debug\net6.0\empty.dll
  empty -> C:\Dev\repro\empty\bin\Debug\net6.0-windows\empty.dll
Test run for C:\Dev\repro\empty\bin\Debug\net6.0\empty.dll (.NETCoreApp,Version=v6.0)
Microsoft (R) Test Execution Command Line Tool Version 17.7.2 (x64)
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

Passed!  - Failed:     0, Passed:     1, Skipped:     0, Total:     1, Duration: < 1 ms - empty.dll (net6.0)
Test run for C:\Dev\repro\empty\bin\Debug\net6.0-windows\empty.dll (.NETCoreApp,Version=v6.0)
Microsoft (R) Test Execution Command Line Tool Version 17.7.2 (x64)
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

Passed!  - Failed:     0, Passed:     2, Skipped:     0, Total:     2, Duration: 2 ms - empty.dll (net6.0)

You can see we have 1 successful test from net6.0 and 2 successful tests from net6.0-windows. You will have to selectively run only the TFMs that are appropriate at runtime with your build scripts, as in my experiment running dotnet test on Linux happily built & ran the net6.0-windows version (and of course it passed, because I didn't actually use any Windows-specific APIs, but I would assume it would fail in real usage).

(When I tried to use net6.0-linux it complained that linux isn't a valid target platform identifier. I'm not sure where these lists come from at this point in time.)

@sliekens
Copy link
Author

sliekens commented Jun 2, 2024

RE:

(When I tried to use net6.0-linux it complained that linux isn't a valid target platform identifier. I'm not sure where these lists come from at this point in time.)

I found this note from the TFM design

Why is there no TFM for Linux?

The primary reason for OS specific TFMs is to vary API surface, not for varying behavior. RIDs allow varying behavior and have support for various Linux flavors. Specifically, TFMs aren't (primarily) meant to allow calling P/Invokes under #if, most of the time that should be done by doing runtime checks or by using RIDs. The primary reason for a TFM is to exclude large amounts of managed representations for OS technologies (WinForms, WPF, Apple's NS APIs, Android etc).

Also, Android, iOS, macOS, and Windows share that they offer a stable ABI so that exchanging binaries makes sense. Linux is too generic of a concept for that, it's basically just the kernel, which again boils down to the only thing you can do is calling P/Invokes.
https://github.com/dotnet/designs/blob/main/accepted/2020/net5/net5.md#why-is-there-no-tfm-for-linux

With that in mind, I don't think OS specific TFMs were intended for this, because conditionally including tests falls into the varying behavior bucket.

I'm still looking at what they mean by "using RIDs". The RID catalog does include linux targets.
https://learn.microsoft.com/en-us/dotnet/core/rid-catalog
https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.NETCore.Platforms/readme.md

@sliekens
Copy link
Author

sliekens commented Jun 2, 2024

I hacked together a solution by pushing behavior into a custom FactAttribute (that always feels a bit wrong to me):

Project:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
    <PackageReference Include="xunit" Version="2.5.3" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.5.3" />
  </ItemGroup>

  <ItemGroup>
    <Using Include="Xunit" />
  </ItemGroup>

</Project>

The custom FactAttribute:

using System.Runtime.InteropServices;

namespace OSPlatformFact;

public sealed class OSPlatformFactAttribute : FactAttribute
{
    public OSPlatformFactAttribute(string platformName)
    {
        var platform = OSPlatform.Create(platformName.ToUpperInvariant());
        if (!RuntimeInformation.IsOSPlatform(platform))
        {
            Skip = $"Test requires {platformName}";
        }
    }
}

Tests which use both the platform compatibility analyzer attributes as well as my custom Fact attribute:

using System.Runtime.Versioning;

namespace OSPlatformFact;

public static class MyMath
{
    [SupportedOSPlatform("windows")]
    public static int Add(int x, int y) => x + y;

    [SupportedOSPlatform("linux")]
    public static int Multiply(int x, int y) => x * y;

    public static int Subtract(int x, int y) => x - y;
}


[SupportedOSPlatform("windows")]
public class WindowsMathTests
{
    [OSPlatformFact("windows")]
    public void Test_only_on_windows()
    {
        Assert.Equal(5, MyMath.Add(3, 2));
    }
}

[SupportedOSPlatform("linux")]
public class LinuxMathTests
{
    [OSPlatformFact("linux")]
    public void Test_only_on_linux()
    {
        Assert.Equal(6, MyMath.Multiply(3, 2));
    }
}

public class MathTests
{
    [Fact]
    public void Test_on_all_platforms()
    {
        Assert.Equal(1, MyMath.Subtract(3, 2));
    }
}

Output of dotnet test on Windows:

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.12]     OSPlatformFact.LinuxMathTests.Test_only_on_linux [SKIP]
  Skipped OSPlatformFact.LinuxMathTests.Test_only_on_linux [1 ms]

Passed!  - Failed:     0, Passed:     2, Skipped:     1, Total:     3, Duration: 10 ms - OSPlatformFact.dll (net8.0)

Output of dotnet test on Linux:

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.28]     OSPlatformFact.WindowsMathTests.Test_only_on_windows [SKIP]
  Skipped OSPlatformFact.WindowsMathTests.Test_only_on_windows [1 ms]

Passed!  - Failed:     0, Passed:     2, Skipped:     1, Total:     3, Duration: 33 ms - OSPlatformFact.dll (net8.0)

I'm still looking for a solution where I only need to add the SupportedOSPlatformAttribrute like in my original post. But I don't know enough about xunit internals to extend it.

@bradwilson
Copy link
Member

I'm still looking for a solution where I only need to add the SupportedOSPlatformAttribrute like in my original post. But I don't know enough about xunit internals to extend it.

I'm not 100% sure myself what the easiest way to do this is right now. I would have to sit down and do some experiments, but surely they would be involved.

FWIW, the skipping version will be easier in v3, because you can just write this as the first line of your test:

Assert.SkipUnless(RuntimeInformation.IsOSPlatform(OSPlatform.Windows), "Test requires Windows");

And we use this today to skip categories of tests which don't support Mono:

Assert.SkipWhen(EnvironmentHelper.IsMono, "Mono does not fully support dynamic assemblies");

As mentioned above, I have an open issue as to whether or not this would be something that could get integrated into the Fact attribute directly: #2339

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants