-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Don't create multiple large files at the same time #62519
Conversation
Tagging subscribers to this area: @dotnet/area-system-io Issue DetailsRelated to #56165 (comment), contributes to #56165 (hopefully) @danmoseley is there any chance you could try this PR on your machine with low disk space? git clone https://github.com/adamsitnik/runtime.git adsitnik && cd adsitnik && git checkout largeFileTests
.\build.cmd -c Release -subset clr+libs+libs.tests
dotnet build /t:Test .\src\libraries\System.IO.FileSystem\tests\System.IO.FileSystem.Tests.csproj /p:Configuration=Release
|
/azp list |
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Sure I'll try it out now. machine's disk space is ~10GB BTW I find |
@adamsitnik it still ran out of disk. you see the files it was trying to create were in the right places:
I had ~9GB free, but these two files filled it up:
I do not understand the time stamps. The tests ran just after 12PM. I looked at one of the files, it was consistent with the writes done in NoInt32OverflowInTheBufferingLogic. The log above shows that TestDirectory is correct. Do we expect temp files to be created as part of the process of writing these large files? BTW does FileCleanupTestBase get disposed every test? I suspect it is every test fixture. So all files created by this class must exist together. |
Hmm, all LargeFileTest failed. So it is not them creating the file. hmm |
And now after cleaning everything, I can't repro it. I suggest to commit this as it's an improvement and I'll look if it happens again. Please do check that the files are deleted between each test though. As it is, they require ~4GB free space each. |
…wn file use single large file to test File.ReadAllBytes and ile.ReadAllBytesAsync for both limits
I've used I've also merged 4 tests that were testing |
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
what?!
|
rerunning failed |
I just meant i clicked the button to rerun them in Azdo. I don't think we need to do more local testing. |
Validation results look good to me. |
I am satisfied that failures are unrelated. |
* move existing large file tests into a separate type (no code changes) * don't run the large file tests in parallel * use FileOptions.DeleteOnClose to ensure that each test removes it's own file use single large file to test File.ReadAllBytes and ile.ReadAllBytesAsync for both limits
) * Move NoInt32OverflowInTheBufferingLogic to OuterLoop and address pending feedback (#60606) * Don't create multiple large files at the same time (#62519) * move existing large file tests into a separate type (no code changes) * don't run the large file tests in parallel * use FileOptions.DeleteOnClose to ensure that each test removes it's own file use single large file to test File.ReadAllBytes and ile.ReadAllBytesAsync for both limits * Fix DisableParallelization build issue * Apply suggestions from code review Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com> * Update src/libraries/System.IO.FileSystem/tests/LargeFileTests.cs Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com> * Notepad with uppercase (#62487) Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com> Co-authored-by: Dan Moseley <danmose@microsoft.com>
Related to #56165 (comment), contributes to #56165 (hopefully)
@danmoseley is there any chance you could try this PR on your machine with low disk space?