geddski / grunt-release

Release a new version of your Node-based project
MIT License
388 stars 121 forks source link

Fix grunt task run fail handling #155

Closed pcdevil closed 8 years ago

pcdevil commented 8 years ago

Hello,
I want to report an issue and request a merge for it's fix as well.

When using afterBump or any option which runs grunt tasks it will be always successful and doesn't respect the return status of the called tasks.

Steps to reproduces:

module.exports = function (grunt) {
    // create some task for test
    grunt.registerTask('yes', 'always successful task', function () {
        console.log('yes called');
        return true;
    });

    grunt.registerTask('no', 'never successful task', function () {
        console.log('no called');
        return false;
    });

    // setup grunt-release
    grunt.config('release', {
        options: {
            bump: true,
            afterBump: [
                'yes',
                'no',
                'yes'
            ],
            file: 'package.json',
            add: true,
            commit: true,

            // we don't need these for the experiment
            push: false,
            tag: false,
            pushTags: false,
            npm: false,
            npmtag: false
        }
    });

    grunt.loadNpmTasks('grunt-release');
};

The task should fail and exit after one no task executed.

Actual result:

The grunt-release task will create the commit after the no task fails.

How to fix

The issues is in the runTask method. The Q.fcall will always resolve the promise because the function doesn't have return value. In my changeset I simplified the method and now it returns all the executed tasks in a Q.all call.

This merge request will fix the issue.
Please review and merge it accordingly!

martinmcnulty commented 7 years ago

This fix doesn't seem to have solved the issue. I created a new module with that Gruntfile, and this package.json, and grunt release creates the commit and succeeds, despite the 'no' task.

package.json:

{
  "name": "grunt-scratch",
  "version": "1.0.10",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "devDependencies": {
    "grunt": "^1.0.1",
    "grunt-release": "^0.14.0"
  }
}

Output:

$ grunt release
Running "release" task
>> running afterBump
>> -> yes
>> -> yes
>> bumped version of package.json to 1.0.11
>> staged package.json
>> Committed all files

Done.

Am I missing something?

pcdevil commented 7 years ago

Hello Martin,
To be real honest I don't use grunt anymore in my daily work so I can't confirm your issue. I've looked into the current state of this method and it's more complex now and I see the for loop called twice. Maybe that causes the issue?

As far as I can tell my modifications never made into production because the Travis CI couldn't find the git tag. So maybe there is no available stable stage of this patch :(

Paging @dorgan as the maintainer of this repo: can you help Martin with his problem?