-
Notifications
You must be signed in to change notification settings - Fork 4.9k
MilliSecond Granualrity added to Copy operation on unix #30996
Conversation
| DateTimeKind.Utc); | ||
| } | ||
|
|
||
| private void CopytoOperation() |
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.
Nit: to => To
| DateTimeKind.Utc); | ||
| yield return TimeFunction.Create( | ||
| ((testFile, time) => CopytoOperation()), | ||
| ((testFile) => testFile.LastWriteTimeUtc), |
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 don't understand how this fits in with this TimeFunction.Create. The purpose of that helper as I understand it is to modify testFile in the first delegate and then validate with the second that it was appropriately changed. But in this new case, testFile isn't being modified in the original delegate, and thus the second delegate is irrelevant. Why is this new test then plugging into this in this way?
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 wanted to use the same behavior if we wanted to add some other similar tests in the future(Checking that the file still has millisecond attribute after some operation).
But i was not able to properly utilize existing this. For time being, i will just move this to its own test.
| FileInfo output = new FileInfo(Path.Combine(TestDirectory, GetTestFileName(), fileName)); | ||
| input.Create().Dispose(); | ||
|
|
||
| Assert.NotEqual(0, input.LastWriteTime.Millisecond); |
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.
Won't this fail 1/1000 times?
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.
yeah you are right. I can just retry and sleep similar to what we do in BaseGetSetTimes
| output = input.CopyTo(output.FullName, true); | ||
|
|
||
| Assert.NotEqual(0, input.LastWriteTime.Millisecond); | ||
| Assert.NotEqual(0, output.LastWriteTime.Millisecond); |
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.
Same here?
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.
Taking care of https://github.com/dotnet/corefx/pull/30996/files/a2c00ef811042b3f32e2bce421b3a1a35209dd2a#diff-9ac637f64c01881307c062f8102f4dc0
will automatically correct this
|
@danmosemsft can you please review this one ? |
| struct timeval origTimes[2]; | ||
| origTimes[0].tv_sec = sourceStat.st_atime; | ||
| origTimes[0].tv_usec = 0; | ||
| origTimes[0].tv_usec = ST_ATIME_NSEC(&sourceStat)/1000; |
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.
Nit, I believe we have single space around arithmetic/logical operators
* test added * Test Modified * Adding Implementation * tabs removed * modifyingTest * Fixing build on HFS * Space around arithemetic operator
* test added * Test Modified * Adding Implementation * tabs removed * modifyingTest * Fixing build on HFS * Space around arithemetic operator
* test added * Test Modified * Adding Implementation * tabs removed * modifyingTest * Fixing build on HFS * Space around arithemetic operator
…x#30996) * test added * Test Modified * Adding Implementation * tabs removed * modifyingTest * Fixing build on HFS * Space around arithemetic operator Commit migrated from dotnet/corefx@387aa6c
Fixes https://github.com/dotnet/corefx/issues/12403
without fix
with fix
The CI should fail on 4f23720 as fix is not there and pass on the recent commit