Code review comment for lp:~benji/testtools/modernize-tsfr

Revision history for this message
Jonathan Lange (jml) wrote :

Hey Benji,

First up, thanks so much for the contribution, and for valiantly persisting in cleaning up this somewhat unclear code. Sadly, I'm afraid it's back to the drawing with this one. I'm afraid it's our fault for not clearly communicating the intent of TSFR.

TSFR's main purpose is to send all of the events for a single test in an atomic unit, preventing the destination result from receiving input like this:

  startTest(foo)
  startTest(bar)
  addSuccess(foo)
  addSuccess(bar)
  stopTest(bar)
  stopTest(foo)

And instead guaranteeing input like this:

  startTest(bar)
  addSuccess(bar)
  stopTest(bar)
  startTest(foo)
  addSuccess(foo)
  stopTest(foo)

If I understand this patch correctly, it wraps each individual event in a semaphore, which isn't enough to guarantee this per-test atomicity.

I'll try to sketch out, or possibly even implement, a solution here.

jml

review: Disapprove

« Back to merge proposal