Mir

Code review comment for lp://qastaging/~afrantzis/mir/fix-1317801

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

* Needs Discussion *

130 + * This is a regression test for bug lp:1317801. This bug is a race and
131 + * very difficult to reproduce with pristine code. By carefully placing
132 + * a delay in the code, we can greatly increase the chances (100% for me)
133 + * that this test catches a regression. However these delays are not
134 + * acceptable for production use, so since the test and code in their
135 + * pristine state are highly unlikely to catch the issue, I have decided
136 + * to DISABLE the test to avoid giving a false sense of security.

I can understand the desire to preserve a recipe for reproducing the problem but I'm not sure that having a disabled test that also requires a patch to the production code is effective.

Would it be less questionable to put a call to a virtual function call into BufferQueue::compositor_acquire() "just before returning the acquired_buffer" with a default null implementation, but override this in a test class with the needed delay?

OTOH any tests that rely on sleep() are unreliable anyway.

review: Needs Information

« Back to merge proposal