Code review comment for lp:~julian-edwards/launchpad/bug-517098

Hi Julian,

This branch is good to land, but I've one question:

> === modified file 'lib/lp/soyuz/tests/test_buildqueue.py'
> --- lib/lp/soyuz/tests/test_buildqueue.py 2010-02-25 16:34:13 +0000
> +++ lib/lp/soyuz/tests/test_buildqueue.py 2010-03-08 16:50:38 +0000
> @@ -96,8 +96,12 @@
> queue_entry.lastscore)
>
>
> -def check_mintime_to_builder(test, bq, min_time):
> +def check_mintime_to_builder(
> + test, bq, min_time, time_stamp=datetime.utcnow()):

Isn't this default a bit fragile? It would be better to declare the
default as None and then have

    if time_stamp is None:
        time_stamp = datetime.utcnow()

in the test, wouldn't it?

> """Test the estimated time until a builder becomes available."""
> + # Monkey-patch BuildQueueSet._now() so it returns a constant time stamp
> + # that's not too far in the future. This avoids spurious test failures.
> + monkey_patch_the_now_property(bq)
> delay = bq._estimateTimeToNextBuilder()
> test.assertTrue(
> delay <= min_time,

Otherwise, r=me.