Skip to content

Commit

Permalink
fix: do not adjust LastAccessTime in FileStream.EndRead (#522)
Browse files Browse the repository at this point in the history
Fix incorrect adjustment of the LastAccessTime in `FileStream.EndRead`
and activate the skipped brittle tests.
Refactor assertions for `DateTime` between two times by adding an
extension method `BeBetween` which checks for the time being between
start and end and applying the system clock tolerance on both values.

*Note: The test keeps failing arbitrarily on MacOS, as the
LastAccessTime is sometimes adjusted, so the test remains skipped on
MacOS*

---------

Co-authored-by: Valentin <[email protected]>
  • Loading branch information
vbreuss and vbreuss authored Mar 26, 2024
1 parent bc86dd0 commit 6a23c3a
Show file tree
Hide file tree
Showing 24 changed files with 199 additions and 285 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,6 @@ public override int EndRead(IAsyncResult asyncResult)
using IDisposable registration = RegisterMethod(nameof(EndRead),
asyncResult);

_container.AdjustTimes(TimeAdjustments.LastAccessTime);
return base.EndRead(asyncResult);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,14 @@ public void CreateDirectory_ShouldAdjustTimes(string path, string subdirectoryNa
if (Test.RunsOnWindows)
{
creationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
lastAccessTime.Should()
.BeOnOrAfter(updateTime.ApplySystemClockTolerance());
}
else
{
lastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

lastWriteTime.Should()
Expand Down Expand Up @@ -168,11 +166,9 @@ public void CreateDirectory_ShouldAdjustTimesOnlyForDirectParentDirectory(
FileSystem.Directory.GetLastWriteTimeUtc(path);

lastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
lastWriteTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}
}

Expand All @@ -184,9 +180,9 @@ public void CreateDirectory_ShouldSetCreationTime(string path)

FileSystem.Directory.CreateDirectory(path);

DateTime end = TimeSystem.DateTime.Now;
DateTime result = FileSystem.Directory.GetCreationTime(path);
result.Should().BeOnOrAfter(start.ApplySystemClockTolerance());
result.Should().BeOnOrBefore(TimeSystem.DateTime.Now);
result.Should().BeBetween(start, end);
result.Kind.Should().Be(DateTimeKind.Local);
}

Expand All @@ -198,9 +194,9 @@ public void CreateDirectory_ShouldSetCreationTimeUtc(string path)

FileSystem.Directory.CreateDirectory(path);

DateTime end = TimeSystem.DateTime.UtcNow;
DateTime result = FileSystem.Directory.GetCreationTimeUtc(path);
result.Should().BeOnOrAfter(start.ApplySystemClockTolerance());
result.Should().BeOnOrBefore(TimeSystem.DateTime.UtcNow);
result.Should().BeBetween(start, end);
result.Kind.Should().Be(DateTimeKind.Utc);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,16 +193,14 @@ public void Delete_ShouldAdjustTimes(string path, string subdirectoryName)
if (Test.RunsOnWindows)
{
creationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
lastAccessTime.Should()
.BeOnOrAfter(updateTime.ApplySystemClockTolerance());
}
else
{
lastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

lastWriteTime.Should()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,16 +147,13 @@ public void Move_ShouldNotAdjustTimes(string source, string destination)
if (Test.RunsOnWindows)
{
creationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

lastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
lastWriteTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

[SkippableTheory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,14 @@ public void AdjustTimes_WhenCreatingAFile_ShouldAdjustTimes(
if (Test.RunsOnWindows)
{
parentCreationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
parentLastAccessTime.Should()
.BeOnOrAfter(updateTime.ApplySystemClockTolerance());
}
else
{
parentLastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

parentLastWriteTime.Should()
Expand All @@ -50,14 +48,11 @@ public void AdjustTimes_WhenCreatingAFile_ShouldAdjustTimes(
DateTime rootLastWriteTime = FileSystem.Directory.GetLastWriteTimeUtc(path1);

rootCreationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
rootLastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
rootLastWriteTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

#if FEATURE_FILESYSTEM_LINK
Expand Down Expand Up @@ -90,16 +85,14 @@ public void AdjustTimes_WhenCreatingASymbolicLink_ShouldAdjustTimes(
if (Test.RunsOnWindows)
{
parentCreationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
parentLastAccessTime.Should()
.BeOnOrAfter(updateTime.ApplySystemClockTolerance());
}
else
{
parentLastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

parentLastWriteTime.Should()
Expand All @@ -110,14 +103,11 @@ public void AdjustTimes_WhenCreatingASymbolicLink_ShouldAdjustTimes(
DateTime rootLastWriteTime = FileSystem.Directory.GetLastWriteTimeUtc(path1);

rootCreationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
rootLastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
rootLastWriteTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}
#endif

Expand Down Expand Up @@ -149,16 +139,14 @@ public void AdjustTimes_WhenDeletingAFile_ShouldAdjustTimes(
if (Test.RunsOnWindows)
{
parentCreationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
parentLastAccessTime.Should()
.BeOnOrAfter(updateTime.ApplySystemClockTolerance());
}
else
{
parentLastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

parentLastWriteTime.Should()
Expand All @@ -169,14 +157,11 @@ public void AdjustTimes_WhenDeletingAFile_ShouldAdjustTimes(
DateTime rootLastWriteTime = FileSystem.Directory.GetLastWriteTimeUtc(path1);

rootCreationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
rootLastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
rootLastWriteTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

[SkippableTheory]
Expand Down Expand Up @@ -207,8 +192,7 @@ public void AdjustTimes_WhenUpdatingAFile_ShouldAdjustTimesOnlyOnWindows(
FileSystem.Directory.GetLastWriteTimeUtc(subdirectoryPath);

parentCreationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
if (Test.RunsOnWindows)
{
parentLastAccessTime.Should()
Expand All @@ -217,12 +201,10 @@ public void AdjustTimes_WhenUpdatingAFile_ShouldAdjustTimesOnlyOnWindows(
else
{
parentLastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

parentLastWriteTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,16 @@ public void LastAccessTime_CreateSubDirectory_ShouldUpdateLastAccessAndLastWrite
if (Test.RunsOnWindows)
{
result.LastAccessTime.Should()
.BeOnOrAfter(sleepTime.ApplySystemClockTolerance());
result.LastAccessTime.Should().BeOnOrBefore(TimeSystem.DateTime.Now);
.BeBetween(sleepTime, TimeSystem.DateTime.Now);
}
else
{
result.LastAccessTime.Should().BeOnOrAfter(start.ApplySystemClockTolerance());
result.LastAccessTime.Should().BeBefore(sleepTime);
result.LastAccessTime.Should()
.BeBetween(start, sleepTime);
}

result.LastWriteTime.Should().BeOnOrAfter(sleepTime.ApplySystemClockTolerance());
result.LastWriteTime.Should().BeOnOrBefore(TimeSystem.DateTime.Now);
result.LastWriteTime.Should()
.BeBetween(sleepTime, TimeSystem.DateTime.Now);
}

[SkippableTheory]
Expand All @@ -110,8 +109,7 @@ public void LastAccessTime_ShouldBeSet(string path)
FileSystem.Directory.CreateDirectory(path);

DateTime result = FileSystem.Directory.GetLastAccessTime(path);
result.Should().BeOnOrAfter(start.ApplySystemClockTolerance());
result.Should().BeOnOrBefore(TimeSystem.DateTime.Now);
result.Should().BeBetween(start, TimeSystem.DateTime.Now);
result.Kind.Should().Be(DateTimeKind.Local);
}

Expand All @@ -124,8 +122,7 @@ public void LastAccessTimeUtc_ShouldBeSet(string path)
FileSystem.Directory.CreateDirectory(path);

DateTime result = FileSystem.Directory.GetLastAccessTimeUtc(path);
result.Should().BeOnOrAfter(start.ApplySystemClockTolerance());
result.Should().BeOnOrBefore(TimeSystem.DateTime.UtcNow);
result.Should().BeBetween(start, TimeSystem.DateTime.UtcNow);
result.Kind.Should().Be(DateTimeKind.Utc);
}

Expand All @@ -138,8 +135,7 @@ public void LastWriteTime_ShouldBeSet(string path)
FileSystem.Directory.CreateDirectory(path);

DateTime result = FileSystem.Directory.GetLastWriteTime(path);
result.Should().BeOnOrAfter(start.ApplySystemClockTolerance());
result.Should().BeOnOrBefore(TimeSystem.DateTime.Now);
result.Should().BeBetween(start, TimeSystem.DateTime.Now);
result.Kind.Should().Be(DateTimeKind.Local);
}

Expand All @@ -152,8 +148,7 @@ public void LastWriteTimeUtc_ShouldBeSet(string path)
FileSystem.Directory.CreateDirectory(path);

DateTime result = FileSystem.Directory.GetLastWriteTimeUtc(path);
result.Should().BeOnOrAfter(start.ApplySystemClockTolerance());
result.Should().BeOnOrBefore(TimeSystem.DateTime.UtcNow);
result.Should().BeBetween(start, TimeSystem.DateTime.UtcNow);
result.Kind.Should().Be(DateTimeKind.Utc);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,14 @@ public void AppendAllText_ShouldAdjustTimes(string path, string contents)
if (Test.RunsOnWindows)
{
creationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
lastAccessTime.Should()
.BeOnOrAfter(updateTime.ApplySystemClockTolerance());
}
else
{
lastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

lastWriteTime.Should()
Expand Down
18 changes: 6 additions & 12 deletions Tests/Testably.Abstractions.Tests/FileSystem/File/CopyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,12 @@ public void Copy_ShouldAdjustTimes(
FileSystem.File.GetLastWriteTimeUtc(destination);

sourceCreationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
if (Test.RunsOnMac)
{
#if NET8_0_OR_GREATER
sourceLastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
#else
sourceLastAccessTime.Should()
.BeOnOrAfter(updateTime.ApplySystemClockTolerance());
Expand All @@ -170,8 +168,7 @@ public void Copy_ShouldAdjustTimes(
else if (Test.RunsOnWindows)
{
sourceLastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}
else
{
Expand All @@ -180,8 +177,7 @@ public void Copy_ShouldAdjustTimes(
}

sourceLastWriteTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
if (Test.RunsOnWindows)
{
destinationCreationTime.Should()
Expand All @@ -190,8 +186,7 @@ public void Copy_ShouldAdjustTimes(
else
{
destinationCreationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

if (!Test.RunsOnMac)
Expand All @@ -201,8 +196,7 @@ public void Copy_ShouldAdjustTimes(
}

destinationLastWriteTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

[SkippableTheory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,11 @@ public void Move_ShouldNotAdjustTimes(string source, string destination)
DateTime lastWriteTime = FileSystem.File.GetLastWriteTimeUtc(destination);

creationTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
lastAccessTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
lastWriteTime.Should()
.BeOnOrAfter(creationTimeStart.ApplySystemClockTolerance()).And
.BeOnOrBefore(creationTimeEnd);
.BeBetween(creationTimeStart, creationTimeEnd);
}

[SkippableTheory]
Expand Down
Loading

0 comments on commit 6a23c3a

Please sign in to comment.