fix: avoid false positives when the failed assertion is not the last one

[jembezmamy]

This MR fixes false positives error that occurs when one of several sync assertions fails:

retry('failed assertion on the last try is not handled', async function (assert, currentRun) {
  assert.ok(false)
  assert.ok(true)
}, 5)

This test should fail, but it passes. It's caused by leveraging last result to determine if the test is successful or not - last result might not be the failed one.

To fix the issue, I propose an alternative approach where isSuccess is updated on each push. That way we make sure it cannot go from false to true on the same run:

- this.lastResult = result
+ this.isSuccess = this.isSuccess && result.result

This approach didn't play well with error rethrowing - these errors didn't trigger pushResult so they didn't update isSuccess correctly. I use pushResult instead.

try {
  await this.callback.call(this.testEnvironment, this.assertProxy, ...this.callbackArgs, this.currentRun)
} catch (err) {
-  if (!this.shouldRetry) {
-    throw err
-  }
+  this.assertProxy.pushResult({
+    result: false,
+    message: err.message
+  })
}

I tried to cover all scenarios to make sure no failed tests are missed. To test a failing test I use retry.todo which is successful when the test fails.

[mrloop]

It feels like isSuccess no longer belongs on the AssertResultHandler as it was derived from a single result, which the AssertResultHandler had knowledge about but now it's from all the results of a single test run. Could the isSuccess state be moved up to the Retry class?

[jembezmamy]

Fixed!

[mrloop]

Thanks!