dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.98k stars 4.66k forks source link

Test failure: Hundreds of tests under 'System.IO.FileSystem.Tests' failed #19847

Closed ghost closed 4 years ago

ghost commented 7 years ago

Opened on behalf of @jiangzeng

Lots of tests of 'System.IO.FileSystem.Tests' failed.

Failing with: System.ArgumentException: Illegal characters in path

Or: System.IO.PathTooLongException: The specified path, file name, or both are too long. The fully qualified file name must be less than 260 characters, and the directory name must be less than 248 characters

Build : Master - 20170108.02 (Full Framework Tests) Failing configurations:

Details: https://mc.dot.net/#/product/netcore/master/source/official~2Fcorefx~2Fmaster~2F/type/test~2Ffunctional~2Fdesktop~2Fcli~2F/build/20170108.02/workItem/System.IO.FileSystem.Tests

ianhays commented 7 years ago

cc: @JeremyKuhne

JeremyKuhne commented 7 years ago

This is the desktop run. The behavior is quite different for < 4.6.2 given long paths (the run targets tfm 4.6). In addition we've opened up legal wildcard characters- this isn't available on any desktop version yet. Looking into what knobs we have available for targeting and validating.

ianhays commented 7 years ago

I figured that was the case. In this situation it would probably be easiest just to mark these tests with [SkipOnTargetFramework(TargetFrameworkMonikers.Net46, "")] or [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "")]

JeremyKuhne commented 7 years ago

Problem is that we have differing behavior for < TFM 4.6.2 and >= 4.6.2. Ideally we should have the appropriate variants up- but I don't know that we have that fine of control yet.

ianhays commented 7 years ago

I imagine in that scenario we only want to test the new behavior, not the old behavior i.e. before 4.6.2. So in that case we would exclude these tests and not worry about adding new tests for the pre-4.6.2 behavior.

e.g.

private const TargetFrameworkMonikers Before462 = TargetFrameworkMonikers.Net45 | TargetFrameworkMonikers.Net451 | TargetFrameworkMonikers.Net452 | TargetFrameworkMonikers.Net46 | TargetFrameworkMonikers.Net461;

[Fact]
[SkipOnTargetFramework(Before462)]
public void test() {}

so when the desktop test run is executed on .net >= 4.6.2 we'll have the coverage.

danmoseley commented 7 years ago

I agree with @ianhays. I don't believe there's value in us maintaining tests against Desktop versions before the latest version. Any breaking changes between Desktop versions, we're presumably already okay with -- and given the choice, Core should match the latest Desktop version.

JeremyKuhne commented 7 years ago

Fixed Argument and PathTooLong exception errors with dotnet/corefx#15100