Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix grunt task run fail handling #155

Merged
merged 1 commit into from
Jun 6, 2016
Merged

Fix grunt task run fail handling #155

merged 1 commit into from
Jun 6, 2016

Conversation

pcdevil
Copy link
Contributor

@pcdevil pcdevil commented May 5, 2016

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:

  • Create this GruntFile.js
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');
};
  • (optional) modify the afterBump with different yes and no order or quantity.
  • Run grunt release

Expected results:

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!

@dorgan dorgan merged commit 0dd2144 into geddski:master Jun 6, 2016
@martinmcnulty
Copy link

martinmcnulty commented Jul 21, 2017

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
Copy link
Contributor Author

pcdevil commented Jul 21, 2017

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants