Skip to content

Return true if nothing has failed in cli#279

Closed
logingood wants to merge 1 commit intoenvato:masterfrom
logingood:master
Closed

Return true if nothing has failed in cli#279
logingood wants to merge 1 commit intoenvato:masterfrom
logingood:master

Conversation

@logingood
Copy link
Copy Markdown

@logingood logingood commented Mar 19, 2019

We should ensure that we return true,
otherwise changes introduced in #276 will cause stack_master to always
exit with code 1, which breaks integration tests.

Addresses issues opened in #277.

cc: @grosser

We should ensure that we return true,
otherwise changes introduced in #276 will cause stack_master to always
exit with code 1, which breaks integration tests.

Addresses issues opened in #277.
Copy link
Copy Markdown

@mesge mesge left a comment

Choose a reason for hiding this comment

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

Add a test? Otherwise 👍

@grosser
Copy link
Copy Markdown
Contributor

grosser commented Mar 19, 2019

this looks wrong to me, the result of execute_stacks_command is not used anywhere afaik

@grosser
Copy link
Copy Markdown
Contributor

grosser commented Mar 19, 2019

#276 made the execute_stacks_command do an exit 1 when any command.perform failed ... so maybe the issue is that some command.perform do not return true ?
if commander fails when true is not returned, then the exit 1 added in #276 should be removed

@grosser
Copy link
Copy Markdown
Contributor

grosser commented Mar 19, 2019

yep ... lib/stack_master/commands/apply.rb does never return true

@logingood
Copy link
Copy Markdown
Author

so just remove OS exit from #276 @grosser and maybe supply a message that stack is missing instead ?

@grosser
Copy link
Copy Markdown
Contributor

grosser commented Mar 19, 2019

The exit is correct in that position afaik, we want exit 1 if a command fails and should not ignore the issue. For some reason an empty apply thinks it's failed ... most likely coming from lib/stack_master/command.rb:26 ... so add a puts caller there to see who triggers it ...

@logingood
Copy link
Copy Markdown
Author

it doesn't go in command.rb:26, will take further look

@logingood
Copy link
Copy Markdown
Author

@grosser it comes from here https://github.com/envato/stack_master/blob/master/bin/stack_master#L14, thing does exit false, which returns status code 1 if there is no true returned by execute! from cli.rb

@logingood
Copy link
Copy Markdown
Author

logingood commented Mar 19, 2019

ruby /tmp/test.rb; echo $?
1
⌂145% [mmukhtarov:~] % cat /tmp/test.rb
#!/usr/bin/ruby

def foo
  puts
end

result = foo
exit !!result

@logingood
Copy link
Copy Markdown
Author

well, I probably getting the problem wrong, anyway would appreciate any help, also will look into tests.

@grosser
Copy link
Copy Markdown
Contributor

grosser commented Mar 19, 2019

#280

@logingood logingood closed this Mar 19, 2019
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