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

Require the 'name' attribute is present in metadata.rb #116

Merged
merged 1 commit into from
May 23, 2013

Conversation

sethvargo
Copy link
Contributor

@fnichol after tracing all day, I found this.

If there's no name attribute in the metadata.rb, TK fails to copy the cookbook, but it doesn't fail until later up the stack.

So I inserted a check... and it still doesn't fail until later up the stack, but the exception is definitely raised. I think we need to link the supervisor/actors together and listen for exceptions on the children?

Basically, I want to give the user a happy little error say "PUT A NAME ATTRIBUTE IF YOUR FREAKING COOKBOOK", but nicer 😄. And instead I get "Message: Failed to complete #converge action: [can't convert nil into String]"

⚠️ not ready for merge

@fnichol
Copy link
Contributor

fnichol commented May 16, 2013

Ah, good idea. Since Berkshelf requires this now and Foodcritic has rules to catch this I suppose we're okay to follow the lead here

@sethvargo
Copy link
Contributor Author

@fnichol right, but it's not being raised. I'm confused...

@@ -158,6 +158,9 @@ def cp_cookbooks(tmpdir)
def cp_this_cookbook(tmpdir)
metadata_rb = File.join(kitchen_root, "metadata.rb")
cb_name = MetadataChopper.extract(metadata_rb).first

raise TestKitchen::Errors::MissingCookbookName.new(name) if cb_name.nil?

cb_path = File.join(tmpdir, cb_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I bet this is the issue: if cb_name = nil and tmdir = '/tmp/blah' then effectively:

File.join('/tmp/blah', nil) # TypeError: can't convert nil into String

My bad…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fnichol right, but this error isn't actually raising :(

fnichol added a commit that referenced this pull request May 23, 2013
Require the 'name' attribute is present in `metadata.rb`
@fnichol fnichol merged commit 023c4d5 into master May 23, 2013
@fnichol fnichol deleted the require_name_attribute branch May 23, 2013 04:33
fnichol added a commit that referenced this pull request May 23, 2013
@fnichol
Copy link
Contributor

fnichol commented May 23, 2013

Got it, the "nil to String" was happening inside an ensure block. Pinned it down in dbfd3c1

@sethvargo
Copy link
Contributor Author

😄

thommay pushed a commit to thommay/test-kitchen that referenced this pull request May 30, 2013
@test-kitchen test-kitchen locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants