Refactor the Reflog entries datetime comparisons

[nulltoken]

We actually rely on Assert.InRange for this and usually ensure the entry has been created within the range of [DateTimeOffset.Now - 5 seconds, DateTimeOffset.Now].

A better alternative (courtesy of @jamill), would be to retrieve the DateTimeOffset.Now before the action generating the reflog entry and ensuring the logged datetime uses this as the lower bound (ie. recordedOffset, DateTimeOffset.Now]).

[nulltoken]

Definitely 👍

I've been seeing such test failures in the past and it happened again with @sitereactor's latest build.

[nulltoken]

This little bugger failed one too many builds. Throwing some code at it.

[nulltoken]

@dahlbyk @jamill Ready for review

[nulltoken]

/cc @martinwoodward

[nulltoken]

/cc @ethomson

[dahlbyk]

👍

[dahlbyk]

I almost wonder if we should capture before in a BaseFixture field. Not sure the slightly better accuracy of inline initialization is worth the extra noise in the tests.

[nulltoken]

@dahlbyk I thought about that, but there are tests that actually perform multiple operations, each of them generating a reflog entry, with a potential different lower boundary. For this kind of cases, we'd need some kind of call to reinitialize what before would contain.

We may end up with some tests having no time related variables at all, magically asserting durations and some others with a strange call to ReinitHiddenTimer() that might be difficult to understand to the reader.

I agree that this adds a ton of noise, but my main goal for this PR was to avoid restarting failed builds because of the way it was previously implemented.

Long story short, I'm very far from being against this proposal, but I'm miserably falling short at imagining a code piece that would make sense. Any ideas?

/cc @jamill

[dahlbyk]

A small objection, let's and revisit if we find the noise unbearable?

[nulltoken]

'd!