Re: [PATCH] Make some commit hashes in tests reproducible

Jeff King [off-list ref] writes:

That's using the same start point as test_tick, though really it could
be anything. I've intentionally _not_ called test_tick at the beginning
of each script, because that would throw off all of the scripts that do
use it by one tick (whereas the first test_tick will overwrite these
values).

Yup, I think that is a sensible approach.

Trying to devil's advocate against this line of reasoning:

  - using the current timestamp introduces more randomness into the test
    suite, which could uncover problems. I'm somewhat skeptical, as the
    usual outcome I see here is that we realize a test's expected output
    is simply racy, and we remove the raciness by using test_tick

True.

  - using the current timestamp could alert us to problems that occur
    only as the clock ticks forward (e.g., if we had a Y2021 bug, we'd
    notice when the clock rolled forward).

True.

  - some tests may rely on having a "recent" timestamp in commits (e.g.,
    when looking at relative date handling). I think all of the
    relative-time tests already use a specific date, though, because
    otherwise we have too many problems with raciness.

True.

Another thing we could do is under DEVELOPER=YesPlease we can set
timestamps you just added here to some random time.

The ones that do care about reproducibility guarantee by using
test_tick would not be affected, and those that were happy with the
current time should be happy with such a random timestamp.  Or we
could just drop what this patch does only under DEVELOPER=YesPlease
which amounts to be the same as setting random time.

Note that the patch above does seem to cause two tests to fail. One of
them I _suspect_ is a raciness problem (order of commits output changes,
which implies the original was expecting the time to increment between
two commits without running test_tick).

Ahh, ok, "git commit && git commit" without these environment
variable set and exported may or may not get the same timestamp from
the wallclock during the testing.  With your patch, they are
guaranteed to get the same timestamp.  On the surface, it sounds
like a valid approach to smoke out any tests that rely on the
passage of time, but realistically the machines are fast enough that
it won't be a rare occasion for these two "git commit" invocations
to be done within a single second, so I am not sure how much we
would be gaining.  We _are_ getting more reproducible test
environment, though, which may be a plus.

On Tue, Jul 07, 2020 at 02:35:57PM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

That's using the same start point as test_tick, though really it could
be anything. I've intentionally _not_ called test_tick at the beginning
of each script, because that would throw off all of the scripts that do
use it by one tick (whereas the first test_tick will overwrite these
values).

Yup, I think that is a sensible approach.

OK, I'll see if I can polish it into a series.

Another thing we could do is under DEVELOPER=YesPlease we can set
timestamps you just added here to some random time.

The ones that do care about reproducibility guarantee by using
test_tick would not be affected, and those that were happy with the
current time should be happy with such a random timestamp.  Or we
could just drop what this patch does only under DEVELOPER=YesPlease
which amounts to be the same as setting random time.

That should be easy to do. I wonder if it's really worth the trouble to
maintain two parallel worlds. I.e., one of the goals would be to stop
worrying about this non-determinism. If we keep it as a requirement,
then we might as well do so all the time (and making it random under
DEVELOPER is effectively keeping it as a requirement, since we expect
all tests to pass under that flag).

I'm also skeptical how often we use system times anyway, because _any_
use of test_commit or test_tick in a script is enough to make all of the
subsequent commands deterministic. I'd be more inclined to let a
particular script say "I'm interested in random times". But then, I'd
think such a script would be better written to trigger its interesting
cases with a well-crafted set of deterministic times.

-Peff