Option to fail on skipped tests

[dmossakowski]

This is a feature request to add ability to fail (exit with non-0 code) on skipped tests. This is to address an issue of someone accidentally using focused Jasmine tests (fdescribe/fit) and checking this into code repository.

The run then exits with:

PhantomJS 2.1.1 (Windows 8.0.0): Executed 1 of 83 (skipped 82) SUCCESS (0.13 secs / 0.059 secs)
TOTAL: 1 SUCCESS

Printing exit code:

$ echo $?
0

This is needed to fail such a run in Maven which uses plugins that rely on exit code to know whether to fail the 'test' goal or not. There is no other known reliable way of providing exit status from Karma. The plugins cannot read output reports as these could be wildly different depending on which report was printed.

Line 72 in frontend-maven-plugin by eirslett:
https://github.com/eirslett/frontend-maven-plugin/blob/master/frontend-plugin-core/src/main/java/com/github/eirslett/maven/plugins/frontend/lib/ProcessExecutor.java

Line 277 in maven-karma-plugin by kelveden:
https://github.com/karma-runner/maven-karma-plugin/blob/master/src/main/java/com/kelveden/karma/StartMojo.java

The feature would be similar to the option to 'failOnEmptyTestSuite' but called something like 'failOnSkippedTests'

Environment Details

• Karma version (output of karma --version):
  Karma version: 3.1.4

  "karma-jasmine": "^2.0.1",
  "karma-phantomjs-launcher": "^1.0.4",
  "karma-htmlfile-reporter": "^0.3.7",
  "karma-jasmine-html-reporter": "^1.4.0",
  "karma-spec-reporter": "0.0.32",

[johnjbarton]

This is a job for a karma 'reporter', that is a plugin that monitors test results. The reporter can set a flag when a spec is skipped, see eg
https://github.com/mlex/karma-spec-reporter/blob/master/index.js#L149
then exit the process with an exitCode in a runComplete, eg
https://github.com/mlex/karma-spec-reporter/blob/master/index.js#L35

[dmossakowski]

I took a closer look and I can see how I can figure out skipped tests from the browsers argument in the reporter. This means that I would need to write my reporter which is doable. I was trying this in karma-spec-reporter/index.js onRunComplete:

  this.onRunComplete = function (browsers, results) {
    //NOTE: the renderBrowser function is defined in karma/reporters/Base.js
	  console.log('karma-spec-reporter index.js - results.skipped:'+ results.skipped)
	  console.log(results)
    this.writeCommonMsg('\n' + browsers.map(this.renderBrowser)
        .join('\n') + '\n');

    if (browsers.length >= 1 && !results.disconnected && !results.error) {
      if (!results.failed) {    	  
    	  browsers.forEach((browser) => {
    	    	console.log('karma-spec-reporter index.js getResults - browser.lastResult.skipped:'+ browser.lastResult.skipped);
    	    	if (browser.lastResult.skipped>0)
    	    	{
    	    		console.log('  found skipped tests so setting to 11')
    	    		results.exitCode+=11; 
    	    	}    	    		
    	    })       	  

Running output:

karma browser_collection.js getResults - browser.lastResult.skipped:82
{ success: 1,
  failed: 0,
  error: false,
  disconnected: false,
  exitCode: 0 }
TOTAL: 1 SUCCESS
karma-spec-reporter index.js - results.skipped:undefined
{ success: 1,
  failed: 0,
  error: false,
  disconnected: false,
  exitCode: 0 }

karma base.js  PhantomJS 2.1.1 (Windows 8.0.0): Executed 1 of 83 (skipped 82) SUCCESS (0.139 secs / 0.066 secs)
karma-spec-reporter index.js getResults - browser.lastResult.skipped:82
  found skipped tests so setting to 11
TOTAL: 1 SUCCESS

Then in console I can see the exit code:

$ echo $?
11

This would be a bit simpler if the 'results' object stored the count of skipped tests but it doesn't:

  getResults (singleRunBrowserNotCaptured, failOnEmptyTestSuite, failOnFailingTestSuite) {
	  
    const results = { success: 0, failed: 0, error: false, disconnected: false, exitCode: 0 }
    this.browsers.forEach((browser) => {
       	console.log('karma browser_collection.js getResults - browser.lastResult.skipped:'+ browser.lastResult.skipped);        
      results.success += browser.lastResult.success
      results.failed += browser.lastResult.failed
      results.error = results.error || browser.lastResult.error
      results.disconnected = results.disconnected || browser.lastResult.disconnected
    })

    results.exitCode = this.calculateExitCode(results, singleRunBrowserNotCaptured, failOnEmptyTestSuite, failOnFailingTestSuite)    
    console.log(results)
    return results
  }

[thw0rted]

I'm a bit confused: it looks like failOnSkippedTests (--fail-on-skipped-tests CLI flag) is in the docs. Maybe it was added at some point, but whoever implemented it forgot to update this ticket? It appears to work, though it's a bit confusing because at least with my config (using reporters: ["kjhtml"] only), I see "TOTAL: NN SUCCESS", no mention of skipped, but the task exits with a failure status. Might be nice if, when this setting causes an error return, it also logged something to console to tell us why, rather than relying solely on configured reporters..

[tarekis]

This should be resolved by #3374 (4ed3af0) as thw0rted mentioned, regarding the reporting of failing on skipped tests however I think the karma infrastructure does not really allow for reporting this by itself, as karma also does not log e.g. failures on failing tests:

karma/lib/browser_collection.js

Lines 41 to 59 in 259be0d

	calculateExitCode (results, singleRunBrowserNotCaptured, config) {
	config = config || {}
	if (results.disconnected || singleRunBrowserNotCaptured) {
	return 1
	}
	if (results.skipped && config.failOnSkippedTests) {
	return 1
	}
	if (results.success + results.failed === 0 && !!config.failOnEmptyTestSuite) {
	return 1
	}
	if (results.error) {
	return 1
	}
	if (config.failOnFailingTestSuite === false) {
	return 0 // Tests executed without infrastructure error, exit with 0 independent of test status.
	}
	return results.failed ? 1 : 0
	}

, it just evaluates the exit code, e.g. for a server here:

karma/lib/server.js

Line 310 in 7a94d33

this.emit('run_complete', singleRunBrowsers, singleRunBrowsers.getResults(singleRunBrowserNotCaptured, config))

and exits with it.
I think this could be done with a reporter tho.

TLDR: Close this issue.