-
Notifications
You must be signed in to change notification settings - Fork 3
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
habitat_package: Add installation_path and dependency_ids properties #32
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Clinton Wolfe <[email protected]>
Signed-off-by: Clinton Wolfe <[email protected]>
…g commands Signed-off-by: Clinton Wolfe <[email protected]>
Signed-off-by: Clinton Wolfe <[email protected]>
Signed-off-by: Clinton Wolfe <[email protected]>
Signed-off-by: Clinton Wolfe <[email protected]>
Signed-off-by: Clinton Wolfe <[email protected]>
# - hab itself is perfect for this. | ||
def determine_hab_install_root | ||
list_result = inspec.backend.run_hab_cli('pkg list core/hab') | ||
hab_spec = list_result.stdout.split("\n").first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stdout.lines.first
is more expressive.
unless match | ||
# TODO: Inspec 3174 resource unable handling | ||
raise Inspec::Exceptions::ResourceFailed, 'Cannot determine habitat install root' | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For something this simple, unless the line is long/ugly, I use an unless modifier. (X unless Y
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was getting confused by this code and match[1]
until my brain flipped on and realized this was just a simple substring regexp match. While =~
is more perl-like, it is more idiomatic ruby and it is measurably faster (it isn't a normal method call and it doesn't create that match object unless it is accessed) so use it and $1
:
path_line =~ %r%="(.+)[\\\/]#{hab_spec}%
raise [...] unless $1
$1
I only flipped it to %r%...%
out of preference. I like that %
has a forward slash so it looks like clean alternative.
{ cmd: 'pkg list core/httpd', stdout_file: 'pkg-list-single.cli.txt' }, | ||
], | ||
general_cli: [ | ||
{ cmd: 'cat /hab/pkgs/core/httpd/2.4.35/20190307151146/TDEPS', stdout_file: 'cat-httpd-tdeps.cli.txt' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these magic strings be pulled out into constants or anything? 🤷♀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably, it references a package version and date, eventually it might not be in the package index.
|
||
# Also, this is really bad, linting-wise; but I'm not sure how to refactor | ||
# this much, in an instance eval.... | ||
def mock_inspec_context_object(test_cxt, fixture) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hah! I love the rubycop:disable
part!
err = fixture[:cli][:stderr_file] ? File.read(File.join(unit_fixture_path, fixture[:cli][:stderr_file])) : '' | ||
run_result.stubs(:stderr).returns(err) | ||
# Boost this to an Array if it isn't already | ||
(fixture[:cli].is_a?(Array) ? fixture[:cli] : [fixture[:cli]]).each do |cli_fixture| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(fixture[:cli].is_a?(Array) ? fixture[:cli] : [fixture[:cli]]).each ...
to:
Array(fixture[:cli]).each ...
run_result = mock | ||
run_result.stubs(:exit_status).returns(cli_fixture[:exit_status] || 0) | ||
|
||
out = cli_fixture[:stdout_file] ? File.read(File.join(unit_fixture_path, cli_fixture[:stdout_file])) : '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor these into a helper method or two.
{ cmd: 'pkg list core/httpd', stdout_file: 'pkg-list-single.cli.txt' }, | ||
], | ||
general_cli: [ | ||
{ cmd: 'cat /hab/pkgs/core/httpd/2.4.35/20190307151146/TDEPS', stdout_file: 'cat-httpd-tdeps.cli.txt' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably, it references a package version and date, eventually it might not be in the package index.
Refs #28
This PR adds three new properties to the
habitat_package
resource:Unit and integration tests, and docs, are included. I suggest starting with the docs.
In addition, the unit test helper was extended to allow simulation of multiple
run_hab_cli
andrun_command
commands. The new code there needs a refactor; see #33 for that.To integration test this, clone the train-habitat repo and switch to the branch with the run_command work, then set a local gem path in the Gemfile.