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

Return error code on failures on actions #300

Merged
merged 2 commits into from
Dec 3, 2021
Merged

Return error code on failures on actions #300

merged 2 commits into from
Dec 3, 2021

Conversation

Pike
Copy link
Contributor

@Pike Pike commented Dec 3, 2021

Fixes #299

@crutchcorn, I tested your suggestion locally, and that did the trick. I took a stab at the test, which seems to test the right thing to me, but YYMV.

Copy link
Member

@crutchcorn crutchcorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One tiny change that I think I'll merge myself (just the test string), but this looks awesome, thanks so much!

Also thank you for putting up with the lack of cli-testing-library's docs. I promise I'm working on that currently

Just curious: what are your initial thoughts on cli-testing-library? Anything you liked, didn't like, thinks needs changing in the API, etc?

tests/action-failure.spec.js Outdated Show resolved Hide resolved
@crutchcorn crutchcorn merged commit 26d61c9 into plopjs:master Dec 3, 2021
@Pike
Copy link
Contributor Author

Pike commented Dec 5, 2021

Just curious: what are your initial thoughts on cli-testing-library? Anything you liked, didn't like, thinks needs changing in the API, etc?

TBH, I don't know. I don't have a ton of expectations on js test infrastructure, I mostly go with the flow of the code base. I guess I was looking for the thing that was used least-or-not-at-all, waiting for the process to finish and then check the result. The fact that that could be done on some output matching was a bit surprising. I initially tried to do it on the result from render, but I also think I didn't try that in the way that ended up working in this patch.

One use-case I have in my head is to use this for plops that have conditional prompts. Not sure if I'll end up giving that a shot, as our use is also rather convoluted UX. Might be better in our case to just separate the generators.

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.

plop command should return non-0 return state when generation failed
2 participants