search

gulpjs / gulp-cli

Command Line Interface for gulp.
MIT License
401 stars 106 forks source link

Fix false positive reporting of sync tasks on errors #204

Closed stof closed 4 years ago

stof commented 4 years ago

When a failure happens in a parallel task, it is totally normal that other tasks have not reported completion, as gulp.parallel will report the error without waiting for other tasks.

Closes #203 Closes #162

phated commented 4 years ago

This solution seems solid but give me a bit to think on it.

Should be able to ship this weekend.

stof commented 4 years ago

test test failure on node 0.10 looks unrelated to my change.

stof commented 4 years ago

and indeed, the latest build on master has even more such timeout failures.

stof commented 4 years ago

@phated did you have a chance to think about it ?

phated commented 4 years ago

@stof thanks for the reminder! Actually, something came to mind this morning: I think we need to support the --continue flag within this change. If someone specifies the --continue flag, we want to continue to report errors and sync tasks because the CLI will won't exit on the first error. Does that make sense?

I'm not sure how hard it will be to thread that flag through the system. It can be specified in the config file too, so we have to thread it from the main function.

stof commented 4 years ago

I would prefer if you could work on adding support for --continue yourselves. I fear I don't know the project good enough to figure out how to get this flag value.

stof commented 4 years ago

@phated AFAIU, even in --continue mode, there is no guarantee that stop or error will be called for each task. Errors are delayed until everything is settled. But then, as single reporting is done in case of an error.

stof commented 4 years ago

@phated PR updated to account for --continue, with 2 new tests to ensure we don't regress on these. Fortunately, detecting this was quite simple, as the sync-tasks logic runs after that process.env.UNDERTAKER_SETTLE is updated based on all possible config, so I could read it (as undertaker does), without having to pass it around explicitly.

phated commented 4 years ago

@stof thanks, looking at this today.

stof commented 4 years ago

@phated any news on this ?

phated commented 4 years ago

@stof I've been really swamped recently. Sorry about that.

I will try to find some time coming up. Thanks for the ping.

phated commented 4 years ago

Thanks for hanging in there! I know it's hard to stay motivated when we take awhile to respond. I'm going to get this out tonight once CI finishes.