Øystein Grøvlen wrote:
> Jørgen Løland wrote:
>> Unfortunately, I think both arguments make sense. I'm not sure if we
>> should scatter the code with thousands of DBUG_EXECUTE_IF's,
>
> That is my point. Adding one does not hurt, but making it a pattern
> would IMO make the code less readable.
>
>> but the test did ensure me that the fix was correct.
>
> Did it really? I think the test ensured you that it was safe for
> doSerialize to return TRUE. The code you fixed is not executed by the
> test. Neither does the test show a case where is evident that
> ignoring a failure in open_tables is wrong.
It assured me that the code that called the function handled the error
correctly. In bug 34867, e.g., it turned out that the corrected return
value was not handled correctly, thus requiring a new patch.
Such tests make perfect sense during development and to make sure that
everything is as expected. In this case, however, it may be overkill and
just add to code complexity if included in the team tree.
--
Jørgen Løland
