Skip to content
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

RenamingABranchIncludesTheCorrectReflogEntries occasionally fails the range check #1186

Open
Vogel612 opened this issue Sep 8, 2015 · 9 comments

Comments

@Vogel612
Copy link
Contributor

Vogel612 commented Sep 8, 2015

Occasionally it happens to me that the Branch Renaming reflog test fails with following error message

 LibGit2Sharp.Tests.BranchFixture.RenamingABranchIncludesTheCorrectReflogEntries: 
     Assert.InRange() Failure
     Range:  (08.09.2015 14:10:34 +02:00 - 08.09.2015 14:10:34 +02:00)
     Actual: 08.09.2015 14:10:33 +02:00

It seems the range's From is off by a second. This behaviour appears only sporadically. As of now I only reproduced this on Ubuntu with Mono-4.0.0.0

The behaviour shows both when running the XUnit Tests from mondevelop as well as when running build.libgit2sharp.sh, but usually disappears when rerunning the test

Behaviour was noticed when working on #1185

@nulltoken
Copy link
Member

😿 We've tried to improve this situation with #999 but your issue hints at the fact that this doesn't solve the issue. Are you seeing this behavior while running the tests from the tip of vNext?

Would you have a proposal for a different (and more successful approach), we'd be very interested.

@Vogel612
Copy link
Contributor Author

Vogel612 commented Sep 8, 2015

I'm seeing this behaviour on the tip of my branch which is up for the mentioned PR... The tests have not changed in the two commits since the tip of vNext. I have verified this behavour happens on the top of vNext, too.

A quick-n-dirty fix may be to just add a second of tolerance, iow. specifying the "from" as 1 second before the actually determined time, but that's really really dirty.

@nulltoken
Copy link
Member

/cc @dahlbyk @jamill @carlosmn Thoughts?

@jamill
Copy link
Member

jamill commented Sep 8, 2015

I wonder where the difference is coming from (2 different mechanisms to get the current time that disagree? maybe there is an issue in our TruncateMilliseconds method)?

@dahlbyk
Copy link
Member

dahlbyk commented Sep 9, 2015

Am I correct in remembering that ref-logging is now handled internal to libgit2?

Can we just subtract a second from before to give ourselves more of a buffer and cross our fingers? Ultimately we're precisely comparing imprecise dates.

@nulltoken
Copy link
Member

Am I correct in remembering that ref-logging is now handled internal to libgit2?

@carlosmn I can't remember, where does the reflog timestamp come from?

@carlosmn
Copy link
Member

When writing the reflog entry, we either take whatever the user has passed in, or we create a new signature from the repository's configuration with the current timestamp. But wall-clock time is weird on computers, and it may go backwards depending on how/who you ask, how recently ntp ran and the current temperature.

While it is somewhat unexpected that the timestamp is before the range, it's not that odd considering libgit2 is asking time(2) and libgit2sharp is asking the C# runtime. Or it could be rounding/truncation or...

@carlosmn
Copy link
Member

One thing I've noticed is that we try to truncate the milliseconds by substracting the leftover ticks per second, but DateTimeOffset does provide both an AddMilliseconds() method as well as Millisecond accessor, so it would seem to be a much more straightforward thing to remove.

@nulltoken
Copy link
Member

@carlosmn I've tried this before, but hit an issue. Removing the elapsed milliseconds keeps some remaining ticks (that do not total to a whole millisecond) and doesn't generate a properly truncated DateTimeOffset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants