mcfly-io / generator-mcfly

A Yeoman generator for scaffolding an application using angular, browserify, ionic and famous
324 stars 43 forks source link

Gulp tests returns 0 when tests fail #240

Closed wrousseau closed 9 years ago

wrousseau commented 9 years ago

Hi there, great generator, loving it so far !

I am unsure if it is happening only to me but it seems that the gulp test tasks always returns 0 including when e2e tests fail. This is an issue for CI services as the badge will always be green even if tests have failed.

jskrzypek commented 9 years ago

Can you upload & provide a link to a repo that demonstrates the problem so we can check it out?

jskrzypek commented 9 years ago

Oh, and thanks for the nice words @wrousseau :beers:

jskrzypek commented 9 years ago

Oh, also another question, do you see this behavior with the karma and mocha unit tests as well?

wrousseau commented 9 years ago

This is actually a private repository but I'll try to create a public one as soon as possible, you don't get this behavior with a freshly generated project at all ?

It seems that this issue does not happen for karma and mocha unit tests (we get 1 if one test fails). I guess it is related to gmux as the e2e task uses it but not the karma task for instance.

jskrzypek commented 9 years ago

I just generated a new project and added a module + controller, fixed the annoying extra comma lint issue that always shows up, and then ran gulp test. The freshly generated app passes the basic e2e test to check the title of the page and exits with 0. I then changed the title and ran gulp test again. The output is pasted below. Here's a link to the repo with the fresh project: https://github.com/jskrzypek/gentest-testfail

And the terminal output:

17:29:13] Using gulpfile ~/dev/testing/gentest-testfail/gulpfile.js
[17:29:13] Starting 'test'...
[17:29:13] Starting 'static'...
[17:29:17] All lint files passed
[17:29:17] Finished 'static' after 3.68 s
[17:29:17] Starting 'lint'...
[17:29:17] Finished 'lint' after 18 μs
[17:29:17] Starting 'karma'...
[17:29:17] Starting Karma server...

Start:
  app.common
    Controllers
      home
        ✔ should be defined
        ✔ should expose controllername

Finished in 0.005 secs / 0.009 secs

SUMMARY:
✔ 2 tests completed
---------------------|----------|----------|----------|----------|----------------|
File                 |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
---------------------|----------|----------|----------|----------|----------------|
 common/             |      100 |      100 |      100 |      100 |                |
  index.js           |      100 |      100 |      100 |      100 |                |
 common/controllers/ |      100 |      100 |      100 |      100 |                |
  home.js            |      100 |      100 |      100 |      100 |                |
  index.js           |      100 |      100 |      100 |      100 |                |
---------------------|----------|----------|----------|----------|----------------|
All files            |      100 |      100 |      100 |      100 |                |
---------------------|----------|----------|----------|----------|----------------|

=============================== Coverage summary ===============================
Statements   : 100% ( 27/27 )
Branches     : 100% ( 0/0 )
Functions    : 100% ( 6/6 )
Lines        : 100% ( 27/27 )
================================================================================
[17:29:25] Finished 'karma' after 7.86 s
[17:29:25] Starting 'mocha'...

  0 passing (1ms)

----------|----------|----------|----------|----------|----------------|
File      |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
----------|----------|----------|----------|----------|----------------|
All files |      100 |      100 |      100 |      100 |                |
----------|----------|----------|----------|----------|----------------|

=============================== Coverage summary ===============================
Statements   : 100% ( 0/0 )
Branches     : 100% ( 0/0 )
Functions    : 100% ( 0/0 )
Lines        : 100% ( 0/0 )
================================================================================
[17:29:25] Finished 'mocha' after 243 ms
[17:29:25] Starting 'e2e'...
[17:29:25] Starting 'webdriver-update'...
[17:29:25] Starting 'dist'...
[17:29:25] Starting 'html'...
[17:29:25] Running task 'html' with targets app in mode dev
[17:29:25] Starting 'html:4Ad4g'...
[17:29:25] Finished 'html:4Ad4g' after 12 ms
[17:29:25] Finished 'html' after 155 ms
[17:29:25] Starting 'image:cordova'...
[17:29:25] Starting 'image:cordova:O0PQ4'...
[17:29:25] Finished 'image:cordova:O0PQ4' after 195 μs
[17:29:25] Finished 'image:cordova' after 17 ms
[17:29:25] Starting 'image'...
[17:29:25] Starting 'image:ql5s3'...
[17:29:25] Finished 'image:ql5s3' after 13 ms
[17:29:25] Finished 'image' after 29 ms
[17:29:25] Starting 'angular:i18n'...
[17:29:25] Starting 'angular:i18n:aM9py'...
[17:29:25] Finished 'angular:i18n:aM9py' after 1.28 ms
[17:29:25] Finished 'angular:i18n' after 15 ms
[17:29:25] Starting 'browserify'...
[17:29:25] Starting 'browserify:gvyhz'...
[17:29:26] gulp-imagemin: Minified 0 images
selenium standalone is up to date.
chromedriver is up to date.
[17:29:26] Finished 'webdriver-update' after 984 ms
[17:29:28] Finished 'browserify:gvyhz' after 2.28 s
[17:29:28] Finished 'browserify' after 2.3 s
[17:29:28] Starting 'font'...
[17:29:28] Starting 'font:cjKon'...
[17:29:28] Finished 'font:cjKon' after 67 ms
[17:29:28] Finished 'font' after 76 ms
[17:29:28] Starting 'style'...
[17:29:28] Starting 'style:Pu3gW'...
[17:29:28] 'css files' main.css 1 B
[17:29:28] Finished 'style:Pu3gW' after 111 ms
[17:29:28] Finished 'style' after 117 ms
[17:29:28] Finished 'dist' after 2.71 s
[17:29:28] Starting 'e2e:serve'...
[17:29:28] Starting 'e2e:serve:nJGC4'...
[17:29:28] Finished 'e2e:serve:nJGC4' after 248 ms
[17:29:28] Finished 'e2e:serve' after 258 ms
[17:29:28] Starting 'e2e:run'...
[17:29:28] Starting 'e2e:run:dhVU1'...
Starting selenium standalone server...
[launcher] Running 1 instances of WebDriver
Selenium standalone server started at http://10.0.0.4:57882/wd/hub
Spec started

  e2e test
    ✗ should have a title (0.995 secs)
      - Expected 'Not the right title' to equal 'Sample app'.

**************************************************
*                    Failures                    *
**************************************************

1) e2e test should have a title
  - Expected 'Not the right title' to equal 'Sample app'.

Executed 1 of 1 spec (1 FAILED) in 1 secs.
Shutting down selenium standalone server.
[launcher] 0 instance(s) of WebDriver still running
[launcher] chrome #1 failed 1 test(s)
[launcher] overall: 1 failed spec(s)
[launcher] Process exited with error code 1
wrousseau commented 9 years ago

I get this as well, but if you run echo $? right afterwards, what do you get ?

jskrzypek commented 9 years ago

Perhaps while you are figuring out the public repo thing, in the meantime you can show me a few things so I can confirm your setup. Can you copy & paste here

jskrzypek commented 9 years ago

Hmm... ok, I see what you mean.

Looking again at the gulp output it looks like gulp doesn't actually see the 'e2e:run' tasks finishing, so I think you do indeed have something here.

wrousseau commented 9 years ago

Alright, that's somehow a good news because I'm not the only one concerned. I've tried to investigate and to check out gmux, but to be perfectly honest, I'm not entirely sure to fully get its purpose :(

jskrzypek commented 9 years ago

Gmux is basically there to facillitate very high levels of code reuse. Imagine you write some angular module for your ionic mobile app, but which is effectively platform agnostic, say one that is purely business logic services, or maybe one that handles the formatting and data brokering of REST calls to your backend. You can copy and past these modules into the separate repo where you are working on your web-based management console, but a) since these shared modules are platform-agnostic and b) since you are using a bundling tool (browserify or webpack) to build & bundle your code anyhow, why not just build to different output paths within the same project? That way you don't need to muck about with git submodules or going back and forth to make sure the common code is always in sync.

This is the idea of gmux and McFly's target system. You use different targets to specify the entry points for the different platforms, and then the bundler handles the rest. You can inject whichever modules you want from anywhere in your app, and you get to reuse all of the code you write. Best of all your tests all run together :)

wrousseau commented 9 years ago

Sounds good indeed, I'll have to dig a bit more in the code to get how it works :)

jskrzypek commented 9 years ago

Can you show me the gulp_tasks/tasks/test.js file you use in your project? I'm able to reproduce the bug but I just want to confirm we have the same version for the task.

wrousseau commented 9 years ago

Sure, here it is https://gist.github.com/wrousseau/e42c403b904efcad8d92

jskrzypek commented 9 years ago

I found it. Apparently browserSync.exit() calls process.exit() for you which kills the whole gulp process and exits with 0. The problem is that if we don't use it at all, the process hangs since browsersync is waiting to exit.

(https://github.com/BrowserSync/browser-sync/blob/master/lib/public/exit.js#L14)

I can't decide if this is our fault for calling browsersync.exit() badly, or if this is a problem with how browsersync handles exiting. I think I'll create an issue with them some time to week to ask.

In the meantime, @wrousseau, the fix for you is simple :)

Just replace your taskE2ERun() function in gulp_tasks/tasks/test.js with this one.

~~var taskE2ERun = function(constants, done) { gulp.src(constants.e2e.src) .pipe($.protractor.protractor({ configFile: constants.e2e.configFile })) .on('error', function(err) { // Make sure failed tests cause gulp to exit non-zero throw err; }) .on('end', function() { // Close connect server to and gulp connect task done(); browserSync.exit(); }); };~~

Calling browserSync.exit() after the done() feels hacky and it kind of is. You get the nice gulp stdout messages like the ones below, but it doesn't actually let you run any tasks afterward. If you have any suggestions about how to do this better, we're all ears!

[20:52:33] Finished 'e2e:run:tND4s' after 8.49 s
[20:52:33] Finished 'e2e:run' after 8.5 s
[20:52:33] Finished 'e2e' after 11 s
jskrzypek commented 9 years ago

Ok I figured out a better non-hacky implementation:

Use this test.js file instead of your old one:

/*eslint no-process-exit:0*/
'use strict';
var gulp = require('gulp');
var runSequence = require('run-sequence');
var $ = require('gulp-load-plugins')();
var mocha = $.mocha;
var istanbul = $.istanbul;
var karma = $.karma;
var browserSync = require('browser-sync').create();
var bs = browserSync;
//var plumber = $.plumber;
var gmux = require('gulp-mux');
var gutil = require('gulp-util');
var chalk = require('chalk');
var args = require('yargs').argv;

var constants = require('../common/constants')();
var helper = require('../common/helper');

gulp.task('mocha', 'Runs mocha unit tests.', function(done) {
    process.env.NODE_ENV = 'mocha';
    gulp.src(constants.mocha.libs)
        .pipe(istanbul({
            includeUntested: true
        }))
        .pipe(istanbul.hookRequire()) // Force `require` to return covered files
        .on('finish', function() {
            gulp.src(constants.mocha.tests)
                .pipe(mocha({
                    reporter: 'spec',
                    globals: constants.mocha.globals,
                    timeout: constants.mocha.timeout
                }))
                .pipe(istanbul.writeReports({
                    reporters: ['lcov', 'json', 'text', 'text-summary', 'cobertura']
                }))
                .once('end', function() {
                    //process.exit();
                    done();
                });
        });
});

gulp.task('karma', 'Runs karma unit tests.', function() {
    return gulp.src(['no need to supply files because everything is in config file'])
        .pipe(karma({
            configFile: 'karma.conf.js',
            action: args.start ? 'start' : 'run',
            autowatch: !args.start,
            debug: args.debug
        })).on('error', function() {
            gutil.log(chalk.red('(ERROR)'), 'karma');
        });
});

gulp.task('unit', 'Runs all unit tests.', function(done) {
    runSequence(
        'lint',
        'karma',
        'mocha',
        done
    );
});

gulp.task('webdriver-update', false, $.protractor.webdriver_update);
//gulp.task('webdriver-standalone', $.protractor.webdriver_standalone);

var taskE2EServe = function(constants, done) {
    var dest = constants.dist.distFolder;
    dest = helper.isMobile(constants) ? dest + '/www' : dest;
    bs = browserSync.init({
        logLevel: 'silent',
        notify: false,
        open: false,
        port: constants.e2e.port,
        server: {
            baseDir: [dest]
        },
        ui: false
    }, done);
};

var taskE2ERun = function(constants, done) {
    gulp.src(constants.e2e.src)
        .pipe($.protractor.protractor({
            configFile: constants.e2e.configFile
        }))
        .on('error', function(err) {
            // Make sure failed tests cause gulp to exit non-zero
            bs.cleanup();
            throw err;
        })
        .on('end', function() {
            // Close connect server to and gulp connect task
            bs.cleanup();
            done();
        });
    gulp.on('stop', process.exit);
};

gulp.task('e2e:run', false, function(done) {
    var taskname = 'e2e:run';
    gmux.targets.setClientFolder(constants.clientFolder);
    if (global.options === null) {
        global.options = gmux.targets.askForSingleTarget(taskname);
    }
    return gmux.createAndRunTasks(gulp, taskE2ERun, taskname, global.options.target, global.options.mode, constants, done);
});

gulp.task('e2e:serve', false, function(done) {
    var taskname = 'e2e:serve';
    gmux.targets.setClientFolder(constants.clientFolder);
    if (global.options === null) {
        global.options = gmux.targets.askForSingleTarget(taskname);
    }
    return gmux.createAndRunTasks(gulp, taskE2EServe, taskname, global.options.target, global.options.mode, constants, done);
});

gulp.task('test', 'Runs all the tests (unit and e2e).', function(done) {
    global.webpackQuiet = true;
    runSequence(
        'lint',
        'karma',
        'mocha',
        'e2e',
        done
    );
});

gulp.task('e2e', 'Runs e2e tests.', function(done) {
    runSequence(
        ['webdriver-update', 'dist'],
        'e2e:serve',
        'e2e:run',
        done
    );
});
wrousseau commented 9 years ago

Very cool, I'll try this tomorrow in the morning and keep you posted ! Thanks for you fast answers and fix !

wrousseau commented 9 years ago

Sorry for the delay for answering ! Works like a charm, thanks a lot !

jskrzypek commented 9 years ago

Awesome, glad to hear!