fix(report): waiting promise resolve in onExist method fix #418

onComplete are async method which should bw complete in onExit method
karma-runner · Jul 22, 2020 · c93f061 · c93f061
1 parent 32acafa
commit c93f061
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 49 deletions.
diff --git a/lib/reporter.js b/lib/reporter.js
@@ -218,58 +218,77 @@ var CoverageReporter = function (rootConfig, helper, logger, emitter) {
     coverageMap.merge(result.coverage)
   }
 
-  this.onRunComplete = async function (browsers, results) {
-    const checkedCoverage = {}
-
-    for (const reporterConfig of reporters) {
-      await Promise.all(browsers.map(async (browser) => {
-        const coverageMap = coverageMaps[browser.id]
-        if (!coverageMap) {
-          return
-        }
+  let checkedCoverage = {}
+  let promiseComplete = null
+
+  this.executeReport = async function (reporterConfig, browser) {
+    const results = { exitCode: 0 }
+    const coverageMap = coverageMaps[browser.id]
+    if (!coverageMap) {
+      return
+    }
 
-        const mainDir = reporterConfig.dir || config.dir
-        const subDir = reporterConfig.subdir || config.subdir
-        const outputPath = generateOutputPath(basePath, browser.name, mainDir, subDir)
-        const remappedCoverageMap = await sourceMapStore.transformCoverage(coverageMap)
-
-        const options = helper.merge(config, reporterConfig, {
-          dir: outputPath,
-          subdir: '',
-          browser: browser,
-          emitter: emitter,
-          coverageMap: remappedCoverageMap
-        })
-
-        // If config.check is defined, check coverage levels for each browser
-        if (hasOwnProperty.call(config, 'check') && !checkedCoverage[browser.id]) {
-          checkedCoverage[browser.id] = true
-          var coverageFailed = checkCoverage(browser, remappedCoverageMap)
-          if (coverageFailed && results) {
-            results.exitCode = 1
-          }
-        }
+    const mainDir = reporterConfig.dir || config.dir
+    const subDir = reporterConfig.subdir || config.subdir
+    const outputPath = generateOutputPath(basePath, browser.name, mainDir, subDir)
+    const remappedCoverageMap = await sourceMapStore.transformCoverage(coverageMap)
+
+    const options = helper.merge(config, reporterConfig, {
+      dir: outputPath,
+      subdir: '',
+      browser: browser,
+      emitter: emitter,
+      coverageMap: remappedCoverageMap
+    })
 
-        const context = istanbulLibReport.createContext(options)
-        const report = reports.create(reporterConfig.type || 'html', options)
+    // If config.check is defined, check coverage levels for each browser
+    if (hasOwnProperty.call(config, 'check') && !checkedCoverage[browser.id]) {
+      checkedCoverage[browser.id] = true
+      var coverageFailed = checkCoverage(browser, remappedCoverageMap)
+      if (coverageFailed && results) {
+        results.exitCode = 1
+      }
+    }
 
-        // // If reporting to console or in-memory skip directory creation
-        const toDisk = !reporterConfig.type || !reporterConfig.type.match(/^(text|text-summary|in-memory)$/)
+    const context = istanbulLibReport.createContext(options)
+    const report = reports.create(reporterConfig.type || 'html', options)
 
-        if (!toDisk && reporterConfig.file === undefined) {
-          report.execute(context)
-          return
-        }
+    // // If reporting to console or in-memory skip directory creation
+    const toDisk = !reporterConfig.type || !reporterConfig.type.match(/^(text|text-summary|in-memory)$/)
 
-        helper.mkdirIfNotExists(outputPath, function () {
-          log.debug('Writing coverage to %s', outputPath)
-          report.execute(context)
-        })
-      }))
+    if (!toDisk && reporterConfig.file === undefined) {
+      report.execute(context)
+      return
     }
+
+    helper.mkdirIfNotExists(outputPath, function () {
+      log.debug('Writing coverage to %s', outputPath)
+      report.execute(context)
+    })
+    return results
   }
 
-  this.onExit = function (done) {
+  this.onRunComplete = function (browsers) {
+    checkedCoverage = {}
+    let results = { exitCode: 0 }
+
+    const promiseCollection = reporters.map(reporterConfig =>
+      Promise.all(browsers.map(async (browser) => {
+        const res = await this.executeReport(reporterConfig, browser)
+        if (res && res.exitCode === 1) {
+          results = res
+        }
+      })))
+    promiseComplete = Promise.all(promiseCollection).then(() => results)
+    return promiseComplete
+  }
+
+  this.onExit = async function (done) {
+    const results = await promiseComplete
+    if (results && results.exitCode === 1) {
+      done(results.exitCode)
+      return
+    }
     if (typeof config._onExit === 'function') {
       config._onExit(done)
     } else {

diff --git a/test/reporter.spec.js b/test/reporter.spec.js
@@ -141,9 +141,9 @@ describe('reporter', () => {
       mkdirIfNotExistsStub.resetHistory()
     })
 
-    it('has no pending file writings', () => {
+    it('has no pending file writings', async () => {
       const done = sinon.spy()
-      reporter.onExit(done)
+      await reporter.onExit(done)
       expect(done).to.have.been.called
     })
 
@@ -298,6 +298,20 @@ describe('reporter', () => {
       expect(mkdirIfNotExistsStub).not.to.have.been.called
     })
 
+    it('should calls done callback when onComplete event will be complete', async () => {
+      reporter = new m.CoverageReporter(rootConfig, mockHelper, mockLogger)
+      reporter.onRunStart()
+      browsers.forEach(b => reporter.onBrowserStart(b))
+      reporter.onRunComplete(browsers)
+      const done = sinon.stub()
+
+      const promiseExit = reporter.onExit(done)
+
+      expect(done.notCalled).to.be.true
+      await promiseExit
+      expect(done.calledOnce).to.be.true
+    })
+
     it('should create directory if reporting text* to file', async () => {
       const run = () => {
         reporter = new m.CoverageReporter(rootConfig, mockHelper, mockLogger)
@@ -435,7 +449,7 @@ describe('reporter', () => {
       expect(options.args[1].watermarks.lines).to.deep.equal(watermarks.lines)
     })
 
-    it('should log errors on low coverage and fail the build', async () => {
+    it.only('should log errors on low coverage and fail the build', async () => {
       const customConfig = helper.merge({}, rootConfig, {
         coverageReporter: {
           check: {
@@ -472,10 +486,13 @@ describe('reporter', () => {
       reporter = new m.CoverageReporter(customConfig, mockHelper, customLogger)
       reporter.onRunStart()
       browsers.forEach(b => reporter.onBrowserStart(b))
-      await reporter.onRunComplete(browsers, results)
+      reporter.onRunComplete(browsers, results)
+
+      const done = sinon.stub()
+      await reporter.onExit(done)
 
       expect(spy1).to.have.been.called
-      expect(results.exitCode).to.not.equal(0)
+      expect(done.calledOnceWith(1)).to.be.true
     })
 
     it('should not log errors on sufficient coverage and not fail the build', async () => {