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

[SHACK-155] cli integration tests #87

Merged
merged 6 commits into from
Apr 20, 2018

Conversation

marcparadise
Copy link
Member

@marcparadise marcparadise commented Apr 20, 2018

I spiked an aruba this morning, and couldn't get it to actually execute the features - it was giving me an internal exception about wrong number of arguments while modeling a test on the example. Rather than continue to shave that yak, I used rspec with a couple of minor helper functions. For what we're doing - looking at complete output of given command - this will work for now. If our needs change, we can revisit it later.

This PR contains the tests themselves, and several minor fixes for issues uncovered while running them.

Future tests will be added individually along with any required fixes.

@marcparadise marcparadise requested a review from a team April 20, 2018 18:43
@marcparadise marcparadise force-pushed the SHACK-155/cli-integration-tests branch 2 times, most recently from 9a11287 to e07b97b Compare April 20, 2018 21:00
@marcparadise
Copy link
Member Author

marcparadise commented Apr 20, 2018

The current test failures are because output contains workstation-specific paths. Looking for ways around that monday morning.

Edit: This is fixed now

@marcparadise marcparadise force-pushed the SHACK-155/cli-integration-tests branch from 470c1fb to 1531773 Compare April 20, 2018 21:19
Copy link
Contributor

@tyler-ball tyler-ball left a comment

Choose a reason for hiding this comment

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

Some minor fixes I found, but looks great overall. We can re-investigate Aruba if we need to

tenor-264412424

if @argv[0].casecmp("help") == 0
# Special case for prefixed --help/-h: we have to move it to the end so that
# we don't consider '-h' to be the command we're trying to load.
if @argv[0].casecmp("help") == 0 || @argv[0] == "-h" || @argv[0] == "--help"
Copy link
Contributor

Choose a reason for hiding this comment

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

if %w{help -h --help}.include?(@argv[0].downcase)

@@ -35,13 +35,13 @@ class Base
option :version,
:short => "-v",
:long => "--version",
:description => T.version,
:description => Text.commands.version.description,
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, where these are not super long I think we don't need to obfuscate with the T reference. I think this (4 levels) is about as long as I am comfortable with

@@ -0,0 +1,21 @@
Chef Workstation Version: 0.1.53
Copy link
Contributor

Choose a reason for hiding this comment

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

$VERSION

@tyler-ball tyler-ball force-pushed the SHACK-155/cli-integration-tests branch from 1531773 to e349339 Compare April 20, 2018 22:34
@tyler-ball tyler-ball merged commit 2b84ecf into master Apr 20, 2018
@tyler-ball tyler-ball deleted the SHACK-155/cli-integration-tests branch April 20, 2018 22:36
@tyler-ball
Copy link
Contributor

@marcparadise I fixed the minor things I found, rebased and squashed what looked like minor commits to me

@marcparadise
Copy link
Member Author

👍 thanks!

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.

2 participants