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

habitat_package: Add installation_path and dependency_ids properties #32

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

clintoncwolfe
Copy link
Contributor

@clintoncwolfe clintoncwolfe commented May 13, 2019

Refs #28

This PR adds three new properties to the habitat_package resource:

  • installation_path
  • dependency_ids
  • dependency_names

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 and run_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.

@clintoncwolfe clintoncwolfe added Type/New Feature Adds new functionality Expeditor/Bump Minor Version CI/CD: Increase the minor version, reset patch Component/Resources labels May 13, 2019
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

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

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)

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' },

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? 🤷‍♀

Copy link
Contributor

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

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|

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])) : ''

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' },
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component/Resources Expeditor/Bump Minor Version CI/CD: Increase the minor version, reset patch Type/New Feature Adds new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants