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

Handle "simple" role dependencies #83

Merged
merged 1 commit into from Nov 3, 2016
Merged

Handle "simple" role dependencies #83

merged 1 commit into from Nov 3, 2016

Conversation

@agx
Copy link
Contributor

agx commented Sep 28, 2016

Dependencies can be

dependencies:
-

or

dependencies:
- { role: }

Handle both.

Copy link
Owner

volanja left a comment

Could you add some test case?

@@ -204,7 +204,7 @@ def self.load_dependencies(role)

if File.exist?(path)
new_deps = YAML.load_file(path).fetch("dependencies", []).map { |h|
h["role"]
h["role"] || h

This comment has been minimized.

Copy link
@volanja

volanja Sep 29, 2016

Owner

Could you add some test case?

@volanja volanja added this to the v0.2.16 milestone Sep 29, 2016
Dependencies can be

  dependencies:
    - <rolename>

or

  dependencies:
    - { role: <rolename> }

Handle both.
@agx agx force-pushed the agx:simple-deps branch from 0069609 to 20f91a1 Sep 29, 2016
@agx
Copy link
Contributor Author

agx commented Sep 29, 2016

On Thu, Sep 29, 2016 at 08:46:58AM -0700, volanja wrote:

volanja requested changes on this pull request.

Could you add some test case?

@@ -204,7 +204,7 @@ def self.load_dependencies(role)

   if File.exist?(path)
     new_deps = YAML.load_file(path).fetch("dependencies", []).map { |h|
  •      h["role"]
    
  •      h["role"] || h
    

Could you add some test case?

I took a short path and made one of the deps in

spec/dependency_spec.rb

a "simple" dep. Hope thats o.k. The test fails without the change and
passes with it. Without:

  1) load_dependencies should correctly resolve nested dependencies
 Failure/Error: path = File.join("./", "roles", role, "meta", "main.yml")

 TypeError:
   no implicit conversion of nil into String
 # ./lib/ansible_spec/load_ansible.rb:203:in `join'
 # ./lib/ansible_spec/load_ansible.rb:203:in `load_dependencies'
 # ./spec/dependency_spec.rb:53:in `block (2 levels) in <top (required)>'

 Finished in 0.47992 seconds (files took 0.52519 seconds to load)
 298 examples, 1 failure

 Failed examples:

rspec ./spec/dependency_spec.rb:56 # load_dependencies should correctly resolve nested dependencies

With change:

Finished in 0.46372 seconds (files took 0.29 seconds to load)
298 examples, 0 failures
@volanja volanja merged commit 4daf774 into volanja:master Nov 3, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@volanja
Copy link
Owner

volanja commented Nov 3, 2016

Sorry for too late response.
I checked your code on my dev-environments, and test case is success.
So I merged it.

volanja pushed a commit that referenced this pull request Nov 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.