Improve time.Time default fuzzer

[lavalamp]

A default fuzzer for time.Time is now in. @thockin changed it to generate times only in the next 1000 years or so; json marshalling has difficulties with negative times and times with more than four digit years.

Should the set of time values we produce include invalid times, or no? There are use cases where people could want either behavior. Users can override this default, but we should document what we decide.

[thockin]

If you want to have gofuzz just generate random times across the whole
spectrum, we can "fix" this problem at the kubernetes layer. Works either
way. I figured it was good enough and not worth much more thought.

On Wed, Mar 4, 2015 at 3:42 PM, Daniel Smith [email protected]
wrote:

A default fuzzer for time.Time is now in. @thockin
https://github.com/thockin changed it to generate times only in the
next 1000 years or so; json marshalling has difficulties with negative
times and times with more than four digit years.

Should the set of time values we produce include invalid times, or no?
There are use cases where people could want either behavior. Users can
override this default, but we should document what we decide.

—
Reply to this email directly or view it on GitHub
#14.

[lavalamp]

Yeah, I don't think it's worth additional thought right now.

[banks]

I found a possibly related issue today. time.Time implements BinaryMarshaller. I'm trying to fuzz a custom codec that relies on that. Currently fuzzTime will generate any uint64 value for nsec which the time.Time does do explicitly say is "valid" although they don't say what is meant by valid!

It seems though that when a value of nsec greater than 999,999,999 is chosen, MarshallBinary will always fail with Time.MarshalBinary: unexpected zone offset (I guess dependent on your local timezone!). At least, that's my understanding!

If I replace the implementation like this:

f := fuzz.New().Funcs(
		func(t *time.Time, c fuzz.Continue) {
			var sec, nsec int64
			// Allow for about 1000 years of random time values, which keeps things
			// like JSON parsing reasonably happy.
			sec = c.Rand.Int63n(1000 * 365 * 24 * 60 * 60)
			nsec = c.Rand.Int63n(999_999_999) // <=== changed line
			*t = time.Unix(sec, nsec)
		},
	)

My test always (so far!) passes. If I use the default one, my test pretty consistently fails (I'm looping 1000 times in the test so the chance of hitting a uint64 greater than a billion is extremely high!)

I'd be very happy to PR that change if others thing this is more useful? It seems the second range was chosen to make JSON parsing happier for a similar reason?

Also happy to open a separate issue if that's deemed necessary since this is a more specific fix than this issue in general!

[lavalamp]

I think that seems OK to me, please send a PR :)

(if people want to fuzz with invalid times, they can write their own function. We could also provide a "WeirdTimeFuzz" function or something to make that extra easy.