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

Should tasks with errors warn or fail? #1163

Open
cowboy opened this issue Jun 18, 2014 · 12 comments
Open

Should tasks with errors warn or fail? #1163

cowboy opened this issue Jun 18, 2014 · 12 comments

Comments

@cowboy
Copy link
Member

cowboy commented Jun 18, 2014

We've seen this issue pop up a bunch of times in the contrib plugins. Some people want a failing task to warn, so that following tasks continue to run. Other people want a failing task to error, so that following tasks don't run.

Currently, the only way to allow the user control over this is to have failing fail, causing any following tasks to not run, which can then be overridden globally with the --force CLI option. Is this how we're currently handling this in contrib?

This seems to be the ideal behavior:

  1. A task should fail on anything that prevents it from accomplishing its primary task.
  2. A task should warn on anything that doesn't prevent it from accomplishing its primary task.
  3. By default, a failing task prevents any further tasks from being run.
  4. A flag should be able to be set in each location the user might specify the task that signifies that it should be exempt from rule 3.

Examples:

  • A linting plugin should fail if there are linting errors. Anything else "bad" is a warning.
  • A compiling plugin should fail if it can't actually compile files (due to a syntax error or file permission error). Anything else "bad" is a warning.
  • A file copying plugin should fail if it can't actually copy files. Anything else "bad" is a warning.

I'd like to discuss these two questions:

  1. How can we settle this once-and-for-all and improve grunt-contrib plugins RIGHT NOW?
  2. How should this problem be solved in the future?
@vladikoff
Copy link
Member

cc @shama

@shama
Copy link
Member

shama commented Jun 18, 2014

1 How can we settle this once-and-for-all and improve grunt-contrib plugins RIGHT NOW?

Fail as you have described but each contrib plugin has a force boolean option to override the individual task (as some plugins already do and people will continue asking for it).

2 How should this problem be solved in the future?

Provide an API that lets users configure how grunt deals with warning/errors. Defaults to the existing behavior but a user can override what should happen on error or warning; globally and per task.

@sindresorhus
Copy link
Member

Pretty much what @shama said.

@cowboy
Copy link
Member Author

cowboy commented Jun 19, 2014

@shama I'd love to chat more about this, are you around at all tomorrow?

@jzaefferer
Copy link
Member

As mentioned in #810, repeated here since no one mentions the exit code:

Whatever the solution here, I'd like you to consider a related, pretty important aspect: The exit code. Currently, setting the force option on tasks that support it will cause grunt to exit with a zero status code, even when one or more forced tasks had errors.

My usecase is pretty simple: Run all tasks, but exit with non-zero status code if anything failed. That way I'd get the most out of Travis builds, where a trailing comma can be printed as an error in the jshint task and causes the build to fail, but it would still run the unit tests.

@mgol
Copy link

mgol commented Jun 19, 2014

👍 to what @jzaefferer says.

@cowboy
Copy link
Member Author

cowboy commented Jun 19, 2014

@jzaefferer what happens when jshint fails because of a syntactic error that prevents the tests from running, or causes them to hang in some way?

@vladikoff
Copy link
Member

That way I'd get the most out of Travis builds, where a trailing comma can be printed as an error in the jshint task and causes the build to fail, but it would still run the unit tests.

@jzaefferer this already does this? See this travis build: JSCS errors, rest of the tests run. Final build fails. https://travis-ci.org/mozilla/fxa-content-server/builds/27542793#L1386

@jzaefferer
Copy link
Member

@vladikoff there's way too much output there to tell what's going on. Last I tried the global ---force option, I got a zero exit code for while tasks had errors.

what happens when jshint fails because of a syntactic error that prevents the tests from running, or causes them to hang in some way?

That should probably still abort the runner with a non-zero exit code, independent of the global --force option.

I think it makes sense to have a force option in plugins. Setting the global --force option will be the same as setting the force option on all plugins. Either way, if at least one task has errors, the overall result should be non-zero.

I don't think there should be a difference between errors and warnings. Allowing warnings that don't cause the build to fail leads to the terrible state where running a build outputs tons of warnings that everyone ignores.

@cowboy
Copy link
Member Author

cowboy commented Jun 19, 2014

@jzaefferer what kind of message do you output when someone uses a deprecated feature or has configured the plugin in a sub-optimal way?

@vladikoff vladikoff added this to the 0.4.7 milestone Jun 19, 2014
@jzaefferer
Copy link
Member

I don't know what "sub-optimal" means here, but throwing an error with a message and stacktrace and aborting execution sounds sane for that kind of issue.

@sbmadhav
Copy link

Is there a way to supress errors or warnings of a task till all other tasks have completed and throw error in the end(collated) ?

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

No branches or pull requests

7 participants