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

omnibus/cli: Exit with non-zero code on failure #822

Merged
merged 1 commit into from
Mar 13, 2023

Conversation

legal90
Copy link
Contributor

@legal90 legal90 commented Feb 10, 2018

Description

Fixes #821

Omnibus CLI inherits the base of Thor, which returns 0 for failed commands by default. Refer to rails/thor#244 for more details.

Copy link
Contributor

@thommay thommay left a comment

Choose a reason for hiding this comment

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

This certainly mirrors the advice on the linked bug. @legal90 i presume you've verified that this works?

@legal90
Copy link
Contributor Author

legal90 commented Feb 12, 2018

@thommay Yes, I've verified:
Both of Example n.2 and Example n.3 from #821 return 1 instead of 0 now:

$ bundle exec omnibus build with something
                    [CLI] I | 2018-02-12T14:46:51+01:00 | Using config from 'omnibus.rb'
ERROR: "omnibus build" was called with arguments ["with", "something"]
Usage: "omnibus build PROJECT"

$ echo $?
1
$ bundle exec omnibus help me
Could not find command "me".

$ echo $?
1

Example n.1 (bundle exec omnibus) still returns 0, but I guess that is an expected behavior in Thor, because that is just a usage call, without actual error occurred.

Copy link
Contributor

@schisamo schisamo left a comment

Choose a reason for hiding this comment

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

Would it be possible to add some Aruba coverage for this change?
https://github.com/chef/omnibus/tree/master/features/commands

@legal90
Copy link
Contributor Author

legal90 commented Feb 12, 2018

@schisamo I would like to, but I don't know how to get it working.

I've added this to build.feature:

  Scenario: When the command contains extra arguments
    When I run `omnibus build bacon eggs`

    Then the output should contain:
         """
         Usage: "omnibus build PROJECT"
         """
    And  the exit status should not be 0

But it fails with a weird error:

$ bundle exec cucumber --color features/commands/build.feature
Feature: omnibus build

  Background:                                      # features/commands/build.feature:2
    Given I have an omnibus project named "hamlet" # features/step_definitions/generator_steps.rb:3

  Scenario: When the project does not exist # features/commands/build.feature:5
    When I run `omnibus build bacon`        # aruba-0.14.2/lib/aruba/cucumber/command.rb:13
    Then the output should contain:         # aruba-0.14.2/lib/aruba/cucumber/command.rb:205
      """
      I could not find a project named `bacon' in any of the project locations:
      """
    And the exit status should not be 0     # aruba-0.14.2/lib/aruba/cucumber/command.rb:277

  Scenario: When the command contains extra arguments # features/commands/build.feature:14
    When I run `omnibus build bacon eggs`             # aruba-0.14.2/lib/aruba/cucumber/command.rb:13
      exit (SystemExit)
      ./lib/omnibus/cli.rb:42:in `execute!'
      features/commands/build.feature:15:in `When I run `omnibus build bacon eggs`'
    Then the output should contain:                   # aruba-0.14.2/lib/aruba/cucumber/command.rb:205
      """
      Usage: "omnibus build PROJECT"
      """
    And the exit status should not be 0               # aruba-0.14.2/lib/aruba/cucumber/command.rb:277

Failing Scenarios:
cucumber features/commands/build.feature:14 # Scenario: When the command contains extra arguments

2 scenarios (1 failed, 1 passed)
8 steps (1 failed, 2 skipped, 5 passed)
0m0.069s

@legal90 legal90 force-pushed the fix-cli-exitcode branch from f66b617 to f9c619e Compare March 8, 2018 12:01
@legal90 legal90 force-pushed the fix-cli-exitcode branch from f9c619e to b797d06 Compare May 2, 2018 07:32
@legal90
Copy link
Contributor Author

legal90 commented May 2, 2018

@thommay @schisamo Do you guys have any suggestions how to fix cucumber error I've posted above?

@legal90
Copy link
Contributor Author

legal90 commented Nov 19, 2018

Hi guys. I'm not interested in this PR anymore since I stopped working on Omnibus.
But I keep this branch available, so feel free to rebase it if you want this bug to be fixed. Cheers!

@scotthain
Copy link
Contributor

Thanks @legal90! Appreciate the efforts you've put in to the project :)

@legal90 legal90 requested review from a team as code owners November 17, 2022 15:21
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@vkarve-chef vkarve-chef merged commit c1b9913 into chef:main Mar 13, 2023
@vkarve-chef
Copy link
Contributor

thanks for the fix, @legal90. we'll take up adding cucumber tests in our backlog.

@legal90 legal90 deleted the fix-cli-exitcode branch June 5, 2023 15:28
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.

Omnibus CLI returns 0 exit code on failed commands
5 participants