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

Overwriting test_base_path to test/recipes instead of test/integration #95

Merged
merged 2 commits into from
Jul 15, 2016

Conversation

tyler-ball
Copy link
Contributor

@tyler-ball tyler-ball commented Jul 14, 2016

This is to support chef-boneyard/chef-dk#934

Long term, this is not the right solution. For example, kitchen-inspec is now ignoring the -t command line flag passed. But we want to get this included in the upcoming ChefDK release and then do the proper fix in the following ChefDK release. See test-kitchen/test-kitchen#1077

\cc @tylercloke @afiune @mwrock

let(:config) {
{
kitchen_root: kitchen_root,
test_base_path: File.join(kitchen_root, 'test', 'integration'),

Choose a reason for hiding this comment

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

Tabbing issue?

@afiune
Copy link

afiune commented Jul 14, 2016

👍

@chris-rock
Copy link
Collaborator

@tyler-ball I understand the use-case as highlighted by @charlesjohnson in chef-boneyard/chef-dk#934. Is there any specific reason why we are not just using:

suites:
  - name: default
    run_list:
      - recipe[apt]
      - recipe[yum]
      - recipe[ssh-hardening]
    verifier:
      inspec_tests:
        - test/recipes

@tyler-ball tyler-ball merged commit d2b2742 into master Jul 15, 2016
@tyler-ball
Copy link
Contributor Author

@chris-rock Because test_base_path is ignored in .kitchen.yml (test-kitchen/test-kitchen#1077). I understand you are saying we could add a new config key and leverage that, but I would prefer to avoid that fragmentation. I think the real solution is to solve the fact that we cannot configure test_base_path in the .kitchen.yml.

@chris-rock
Copy link
Collaborator

@tyler-ball How would the migration work? Currently a lot of users use the test/integration directory already. In addition I am not sure, how we are handling our support for puppet and ansible then. kitchen-inspec should act independent from any configuration management tool eg. if users run the shell provisioner, kitchen-inspec should still work perfectly. My feeling is, that it would sounds strange if we would use test/recipe for other cfgmgmt tools than chef. Any ideas?

@chris-rock
Copy link
Collaborator

Maybe that spins up the question, that we should not assume any default directory and we have to name the test-directory all the time, e.g. like:

suites:
  - name: default
    run_list:
      - recipe[apt]
      - recipe[yum]
      - recipe[ssh-hardening]
    verifier:
      inspec_tests:
        - test/integration

That would also make the behavior in kitchen-inspec more obvious, since we already support to use multiple test locations. The deprecation of test/integration as default location would help to make it very obvious for users, where tests are located, since it will be named in .kitchen.yml all time

@mwrock
Copy link
Contributor

mwrock commented Jul 15, 2016

@tyler-ball the point about migrations from @chris-rock above is important. Unless I'm missing something, this will break all the things since everyones tests are currently under integration. we should have a fallback.

@mwrock
Copy link
Contributor

mwrock commented Jul 15, 2016

@chris-rock just my 2 cents - I like having a default test location and not having to specify a location for simplicity. But makes sense to provid a way to override.

@tyler-ball
Copy link
Contributor Author

Just to be clear - tests in test/integration will still continue to work perfectly fine. Look at the Inspec tests for kitchen-inspec - there are both test/integration tests and test/recipes tests. Both are supported (in parallel even). I made sure to not break existing cookbooks. And if there is some existing cookbook case I am not thinking of that I broke, please let me know so I can release a patch version.

@mwrock
Copy link
Contributor

mwrock commented Jul 15, 2016

cool @tyler-ball and thats why you rock!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants