Static asset compilation errors are ignored about capistrano-magento2 HOT 8 CLOSED

@davidalger I look forward to reviewing your PR. I won't be able to review it until early next week, but when I do, I can test it locally to ensure it works with the specific use that prompted me to create this issue.

from capistrano-magento2.

I accidentally created this task without a body. Just updated it with content.

from capistrano-magento2.

I just did some tests and it actually appears that the bin/magento setup:static-content:deploy command returns a non-zero exit code ("1") when there are compilation errors. Take a look at this code and you'll see it references this issue. Per @davidalger's latest comment, it appears this is now in place in 2.1.0+.

@davidalger In order to fully support 2.0.x, we'll need to do two checks:

For 2.0.x, revert to checking for "Compilation from source"
For 2.1.0+, switch from the current check for "New version of deployed files" to checking the return code of the setup:static-content:deploy command.

I wonder how much longer capistrano-magento2 should continue to support 2.0…

from capistrano-magento2.

I wonder how much longer capistrano-magento2 should continue to support 2.0…

For as long as 2.0 is a supported major release ;)

For 2.1.0+, switch from the current check for "New version of deployed files" to checking the return code of the setup:static-content:deploy command.

This is actually supposed to be automatic. If the exit code of a command is non-zero, the deploy fails…I just verified this is the case with capture as well as execute where it's fairly well assumed. How did you check the error codes the console returns when receiving this error? I can't seem to get the command to throw any errors for my immediate testing. Can you run the command on the broken code and then run echo $? on the CLI to see what the return code is?

For 2.0.x, revert to checking for "Compilation from source"

I believe the reason I switched away from this in the first place is because that is only one potential error it could spit out. Whats to say there couldn't be other error related strings in the output? Hence the switch to a positive check, which I assumed? worked when I implemented it having no broken code to test with :) based on the output above, this assumed to be positive message shows regardless of there being errors or not.

I'm considering implementing a check for the number of errors reported. It would iterate over each line and check the errors: 123 text for a number greater than zero. This should catch it regardless of the console exit code.

from capistrano-magento2.

@erikhansen See above pushed commit for the type of implementation I'm suggesting. Open to any feedback. Want to make sure I get it right this time! ;)

from capistrano-magento2.

@erikhansen I've made some additional changes and submitted a PR to develop. Please review. The positive check for presence of the "New version…" text is necessary, as it won't be there (and neither will the "errors: xyz" text) in the case of an Exception being thrown. This can be easily seen if you (for example) run the console command with an invalid option triggering a RuntimeException.

from capistrano-magento2.

@davidalger I checked out your issue-53 branch locally, ran bundle exec rake install to deploy it locally, and went through the same deployment steps that prompted the creation of this task. I'm happy to report that Capistrano did catch the compilation error:

FYI, I did not test this on a 2.0.x install.

from capistrano-magento2.

The changes have been pushed up as part of v0.5.4.

from capistrano-magento2.