Re: [Qemu-devel] [PATCH] Fix conversion between 12 hours and 24 hours modes.

[Paolo Bonzini]

Il 15/02/2013 16:41, Antoine Mathys ha scritto:
> On 02/15/2013 12:24 PM, Andreas Färber wrote:
>> The expected answer would've been "take guest X and do Y to see Z", or
>> better to extend the existing qtest cases to prove something was broken
>> before and fixed afterwards and to avoid the same bug being reintroduced
>> later.
> 
> If we are talking about adding a test case in order to have some
> guarantee that what works after a fix keeps working in the future,
> that's fine. And I am willing to add such tests for the DS1338
> implementation (once it is finished).
> 
> But demanding a test case that the code passes with the fix but fails
> without, in order to prove that something was broken before, is only
> reasonable if the bug was found through testing in the first place.

It depends.  For example, the mc146818rtc model was rewritten almost
completely last year.  Testcases helped a lot, and more testcases would
have helped even more.  It does not matter if they came from past bugs,
or from code review, or from blackbox testing.

> Not only do you
> not need a test case to prove the bug exists, but reverse-engineering a
> test-case can be a significant undertaking.

That's true.  But...

> Paolo tried to do that
> unsuccessfully in the case of bug 1090558 and I had no reason to think I
> could do better. This does not strike me as a very productive use of
> developer time anyway.

... at least trying to do that _is_ a productive use of developer time.
 Of course, insisting at all costs is not.  So a reviewer is right in
asking for a testcase and complaining of a lack of testcases.  He should
be okay with "I couldn't find one" just as well, especially if it comes
with a patch that actually adds testcases.  My effort to reproduce bug
1090558 did produce such a patch.

As an aside ds1338 is a much simpler model than mc146818rtc (the
"capture" behavior is much nicer than mc146818rtc's UIP, and ds1338 also
has no alarm and no interrupts), the bug should be almost trivial to
test for.

> And your suggestion that it is better to leave a known bug unpatched
> until someone can conjure up a test case is ridiculous. I don't see how
> that attitude help users, in the short or long term.

Of course some common sense is in order, as usual.  Leaving bugs
unpatched is bad, but prodding contributors and other maintainers is good.

Paolo