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.

Sign up for GitHub

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

Jump to bottom

tests: Fix profile_context_test NameError #3758

Merged
merged 2 commits into from Jan 31, 2019
Merged

tests: Fix profile_context_test NameError #3758

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8520859
Add missing require to Inspec::DSL#missing_method
jerryaldrichiii Jan 26, 2019
72c57c6
Move `require` and fix load order issue
jerryaldrichiii Jan 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/inspec/dsl.rb
View file
Open in desktop
Expand Up @@ -31,6 +31,7 @@ def require_resource(options = {})
# This is called when an unknown method is encountered
# "bare" in a control file - outside of a control or describe block.
def method_missing(method_name, *arguments, &block)
require 'inspec/plugin/v2'
Copy link
Contributor

Choose a reason for hiding this comment

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

@jerryaldrichiii, can you explain why this location was chosen for the require? It would seem more typical to require in the top of lib/inspec/dsl.rb, or in lib/inspec/profile_context.rb (which is where the error appeared).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great call out. I could use some words on describing the behavior succinctly. I believe the issue is because we are doing an instance_eval. The only places I could get it to work were to place the require here: https://github.com/inspec/inspec/blob/master/lib/inspec/profile_context.rb#L152 or the place you see it. I did the latter since it is closer to

inspec/lib/inspec/dsl.rb

Line 36 in 8520859

registry = Inspec::Plugin::V2::Registry.instance

Placing at the top of the files like you would expect leads me down a rabbit hole that I couldn't get to the bottom of (issues like not being able to find Inspec::BaseCLI).

Copy link
Contributor

Choose a reason for hiding this comment

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

If you'd like to pair or group work on this, I'd be happy to do so; I'm familiar with the missing require for BaseCLI from the plugin/v2/plugin_types/cli type. Simple fix there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh? Great! I'll bring it up in standup and we can get some time scheduled if the usual parking lot time doesn't cover it.

# Check to see if there is a outer_profile_dsl plugin activator hook with the method name
registry = Inspec::Plugin::V2::Registry.instance
hook = registry.find_activators(plugin_type: :outer_profile_dsl, activator_name: method_name).first
Expand Down