-
Notifications
You must be signed in to change notification settings - Fork 565
[tests] add checks if long paths supported #3543
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
Conversation
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs
Outdated
Show resolved
Hide resolved
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs
Outdated
Show resolved
Hide resolved
6c4f171 to
dba0964
Compare
bec3165 to
878aec0
Compare
|
To see a build where I forced the Windows tests to run on the broken agent: http://build.azdo.io/2985000
The key seemed to be using 255 as a length for the filename, since there is still a limit on file name length in addition to
https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file#maximum-path-length-limitation |
| Assert.Ignore ("This environment supports long paths"); | ||
| } | ||
| var file = NewFile (fileName: "foo".PadRight (250, 'N')); | ||
| var file = NewFile (fileName: "foo".PadRight (255, 'N')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be going slightly toward asinine territory, but should we P/Invoke GetVolumeInformation() (pinvoke.net) to get the appropriate maximumComponentLength value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, should this use FIlesTests.MaxFileName for consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the p/invoke is probably taking it too far, I moved this const to BaseTest so it could just be used everywhere.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BaseTest.cs
Outdated
Show resolved
Hide resolved
Context: http://build.azdo.io/2971289 Our `MaxPathTests` seem to be failing on a specific build machine: `DDMBLDW137`. I suspect this machine has long paths enabled. I added a check that attempts to write a long path, and if it succeeds we can `Assert.Ignore` these tests appropriately. A filename can only be 255 characters, so I added a constant for this value so the various tests can use it.
Temporarily require `$(Agent.Name)` = `DDMBLDW137`
This reverts commit 878aec0.
e62890b to
4080e03
Compare
Context: http://build.azdo.io/2971289 Our `MaxPathTests` (a3009f7) seem to be failing on a specific build machine: DDMBLDW137. I suspect this machine has long paths enabled. I added a check that attempts to write a long path, and if it succeeds we can `Assert.Ignore()` these tests appropriately. A filename can only be 255 characters, so I added a constant for this value so the various tests can use it.


Context: http://build.azdo.io/2971289
Our
MaxPathTestsseem to be failing on a specific build machine:DDMBLDW137. I suspect this machine has long paths enabled.I added a check that attempts to write a long path, and if it succeeds
we can
Assert.Ignorethese tests appropriately.