Merge pull request #17 from mrloop/fix-false-positives

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

src/assert-result-handler.js

@@ -15,13 +15,9 @@ export default class AssertResultHandler {
     return `${message}(Retried ${retryNum} times)`
   }
 
-  get isSuccess () {
-    return this.lastResult && this.lastResult.result
-  }
-
   pushResultFn (target) {
     return (result) => {
-      this.lastResult = result
+      this.retry.isSuccess = this.retry.isSuccess && result.result
       if (!this.retry.shouldRetry) {
         result.message = this.retryMessage(result.message, this.retry.currentRun)
         target.pushResult(result)

src/retry.js

@@ -6,6 +6,7 @@ export default class Retry {
     this.callback = callback
     this.maxRuns = maxRuns
     this.assertResultHandler = new AssertResultHandler(this)
+    this.isSuccess = true
 
     testFn(...args, async (assert, ...callbackArgs) => {
       this.assertProxy = new Proxy(assert, this.assertResultHandler)
@@ -16,7 +17,7 @@ export default class Retry {
   }
 
   get shouldRetry () {
-    return this.currentRun < this.maxRuns && !this.assertResultHandler.isSuccess
+    return this.currentRun < this.maxRuns && !this.isSuccess
   }
 
   get test () {
@@ -59,6 +60,7 @@ export default class Retry {
 
   async runTest () {
     if (this.notFirstRun) {
+      this.isSuccess = true
       this.test.testEnvironment = extend({}, this.test.module.testEnvironment, false, true)
       await this.runHooks(this.beforeEachHooks)
     }
@@ -73,9 +75,10 @@ export default class Retry {
     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
+      })
     }
   }
 

test/retry.js

@@ -42,12 +42,45 @@ retry('test retry async', async function (assert, currentRun) {
   assert.equal(currentRun, 4)
 }, 4)
 
-retry('promise reject', async function (assert, currentRun) {
-  if (currentRun === 2) {
-    await Promise.reject(new Error('should be handled'))
-  }
-  assert.equal(currentRun, 5)
-}, 5)
+QUnit.module('error handling', function () {
+  retry('rejected promise is handled', async function (assert, currentRun) {
+    if (currentRun === 2) {
+      await Promise.reject(new Error('should be handled'))
+    }
+    assert.equal(currentRun, 5)
+  }, 5)
+
+  retry.todo('rejected promise on the last try is is not handled', async function (assert, currentRun) {
+    if (currentRun === 5) {
+      await Promise.reject(new Error('should not be handled'))
+    }
+    assert.equal(currentRun, 5)
+  }, 5)
+
+  retry('error is handled', async function (assert, currentRun) {
+    if (currentRun === 2) {
+      throw new Error('should be handled')
+    }
+    assert.equal(currentRun, 5)
+  }, 5)
+
+  retry.todo('error on the last try is not handled', async function (assert, currentRun) {
+    if (currentRun === 5) {
+      throw new Error('should not be handled')
+    }
+    assert.equal(currentRun, 5)
+  }, 5)
+
+  retry('failed assertion is handled', async function (assert, currentRun) {
+    assert.equal(currentRun, 5)
+    assert.ok(true)
+  }, 5)
+
+  retry.todo('failed assertion on the last try is not handled', async function (assert, currentRun) {
+    assert.ok(false)
+    assert.ok(true)
+  }, 5)
+})
 
 QUnit.module('hook context', function (hooks) {
   hooks.beforeEach(function () {