Skip to content

Akka.Streams: fix race conditions with synchronous file sink specs #6565

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

Merged

Conversation

Aaronontheweb
Copy link
Member

@Aaronontheweb Aaronontheweb commented Mar 24, 2023

Changes

Akka.Streams.Tests.IO.FileSinkSpec.SynchronousFileSink_should_allow_appending_to_file and other specs frequently fail with errors such as:

System.AggregateException : One or more errors occurred. (The process cannot access the file 'C:\Users\VssAdministrator\AppData\Local\Temp\synchronous-file-sink.tmp' because it is being used by another process.)
---- System.IO.IOException : The process cannot access the file 'C:\Users\VssAdministrator\AppData\Local\Temp\synchronous-file-sink.tmp' because it is being used by another process.

This is because we use a static file name and other tests might not be finished fully disposing / disposed cleanly when trying to access this file. Now we randomize the file name on each individual test in order to keep them isolated.

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

@@ -444,7 +444,7 @@ public void SynchronousFileSink_should_write_buffered_element_if_manual_flush_is
ActorMaterializer materializer,
bool create = true)
{
var targetFile = new FileInfo(Path.Combine(Path.GetTempPath(), "synchronous-file-sink.tmp"));
var targetFile = new FileInfo(Path.Combine(Path.GetTempPath(), $"synchronous-file-sink-{Guid.NewGuid()}.tmp"));
Copy link
Member Author

Choose a reason for hiding this comment

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

Append a new Guid for unique randomization

@Aaronontheweb Aaronontheweb enabled auto-merge (squash) March 24, 2023 16:32
Copy link
Contributor

@Arkatufus Arkatufus left a comment

Choose a reason for hiding this comment

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

LGTM, waiting for tests to pass

@Aaronontheweb
Copy link
Member Author

This one is still failing

Akka.Streams.Tests.IO.FileSinkSpec.SynchronousFileSink_should_by_default_replace_the_existing_file

Leads me to believe there's an issue with the spec design itself there.

@Aaronontheweb Aaronontheweb merged commit d1299d0 into akkadotnet:dev Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants