cookbook created by chef generate cookbook fails foodcritic due to test directory #1027

mattstratton · 2016-09-25T03:36:25Z

Description

After creating a skeleton cookbook with chef generate cookbook, it results in a cookbook that fails foodcritic validation due to some of the files in the test/ directory.

Example:

FC011: Missing README in markdown format: test/README.md:1
FC023: Prefer conditional attributes: ./test/recipes/default_test.rb:8
FC031: Cookbook without metadata file: test/metadata.rb:1
FC045: Metadata does not contain cookbook name: test/metadata.rb:1

ChefDK Version

ChefDK version:

Chef Development Kit Version: 0.17.17
chef-client version: 12.13.37
delivery version: master (f68e5c5804cd7d8a76c69b926fbb261e1070751b)
berks version: 4.3.5
kitchen version: 1.11.1

Platform Version

OS X Sierra

Replication Case

To replicate, create a new cookbook using chef generate cookbook and then execute foodcritic . in that directory.

The text was updated successfully, but these errors were encountered:

qubitrenegade · 2016-09-29T17:54:48Z

We are experiencing this same issue on CentOS 6 with ChefDK 18.30.

I have to ask, why are we doing away with the test/integration/<test suite> model? For instance, I have a cookbook that installs a vendor package either via rpm or via tarball. In each case I have different integration tests that I want to run. It's not appropriate to test that we downloaded the tarball when installing the RPM version. Under ServerSpec this is easy, all I have to do is specify which suite I want to run. However, unless I'm misunderstanding, this seems to lump all InSpec integration tests into the same directory.

Another use case might be one cookbook that runs on different operating systems. If my cookbook runs on both Windows and Linux it's probably not appropriate to check for the existence of config files in /etc/blah on a Windows machine.

Wouldn't it make more sense and ease adoption if all the InSpec tests went in test/integration/default/inspec? This could allow InSpec and ServerSpec tests to live together while we transition from ServerSpec to InSpec.

Thanks!
Jon A

vinyar · 2016-10-04T21:54:24Z

+1
This is causing delivery lint phase to explode
What's happening is foodcritic thinks /test is a cookbook of its own, and it all goes sideways from there.

Synchronizing Cookbooks:
  - delivery-truck (2.2.2)
  - build_cookbook (0.1.0)
  - delivery-sugar (1.1.3)
Installing Cookbook Gems:
Compiling Cookbooks...
Converging 2 resources
Recipe: delivery-truck::lint
  * execute[lint_foodcritic_staging] action run

    ================================================================================
    Error executing action `run` on resource 'execute[lint_foodcritic_staging]'
    ================================================================================

    Mixlib::ShellOut::ShellCommandFailed
    ------------------------------------
    Expected process to exit with [0], but received '3'
    ---- Begin output of foodcritic -f correctness   /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/ ----
    STDOUT: FC011: Missing README in markdown format: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/test/README.md:1
    FC023: Prefer conditional attributes: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/test/recipes/default_test.rb:8
    FC031: Cookbook without metadata file: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/test/metadata.rb:1
    FC045: Metadata does not contain cookbook name: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/test/metadata.rb:1
    FC064: Ensure issues_url is set in metadata: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/metadata.rb:1
    FC065: Ensure source_url is set in metadata: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/metadata.rb:1
    STDERR: 
    ---- End output of foodcritic -f correctness   /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/ ----
    Ran foodcritic -f correctness   /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/ returned 3

    Resource Declaration:
    ---------------------
    # In /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/cache/cookbooks/delivery-truck/recipes/lint.rb

     20:   execute "lint_foodcritic_#{cookbook.name}" do
     21:     command "foodcritic #{foodcritic_fail_tags} #{foodcritic_tags} " \
     22:       "#{foodcritic_excludes} #{cookbook.path}"
     23:   end
     24: 

    Compiled Resource:
    ------------------
    # Declared in /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/cache/cookbooks/delivery-truck/recipes/lint.rb:20:in `block in from_file'

    execute("lint_foodcritic_staging") do
      action [:run]
      retries 0
      retry_delay 2
      default_guard_interpreter :execute
      command "foodcritic -f correctness   /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/"
      backup 5
      returns 0
      declared_type :execute
      cookbook_name "delivery-truck"
      recipe_name "lint"
    end

    Platform:
    ---------
    x86_64-linux


Running handlers:
[2016-10-04T21:38:00+00:00] ERROR: Running exception handlers
Running handlers complete
[2016-10-04T21:38:00+00:00] ERROR: Exception handlers complete
Chef Client failed. 0 resources updated in 06 seconds
[2016-10-04T21:38:00+00:00] FATAL: Stacktrace dumped to /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/cache/chef-stacktrace.out
[2016-10-04T21:38:00+00:00] FATAL: Please provide the contents of the stacktrace.out file if you file a bug report
[2016-10-04T21:38:00+00:00] ERROR: execute[lint_foodcritic_staging] (delivery-truck::lint line 20) had an error: Mixlib::ShellOut::ShellCommandFailed: Expected process to exit with [0], but received '3'
---- Begin output of foodcritic -f correctness   /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/ ----
STDOUT: FC011: Missing README in markdown format: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/test/README.md:1
FC023: Prefer conditional attributes: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/test/recipes/default_test.rb:8
FC031: Cookbook without metadata file: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/test/metadata.rb:1
FC045: Metadata does not contain cookbook name: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/test/metadata.rb:1
FC064: Ensure issues_url is set in metadata: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/metadata.rb:1
FC065: Ensure source_url is set in metadata: /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/metadata.rb:1
STDERR: 
---- End output of foodcritic -f correctness   /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/ ----
Ran foodcritic -f correctness   /var/opt/delivery/workspace/ip-10-0-0-67.ec2.internal/alex_ent/twenty/staging/master/verify/lint/repo/ returned 3
[2016-10-04T21:38:01+00:00] FATAL: Chef::Exceptions::ChildConvergeError: Chef run process exited unsuccessfully (exit code 1)

vinyar · 2016-10-04T22:00:26Z

looks like this issue has been around for ever.. Foodcritic/foodcritic#148

vinyar · 2016-10-05T21:20:15Z

@mattstratton did you see it as part of workflow?

I ran into the same thing, and the bandaid is to use use config.json. I am curious about having a conversation around what's the correct place to fix it - probably foodcritic as default behavior should not scan non-cookbook files

mattstratton · 2016-10-05T21:32:22Z

@vinyar nope. Just doing straight CLI foodcritic. I'm writing something up about chefdk and testing tools, without the context of Workflow, so config.json won't help.

vinyar · 2016-10-05T21:53:54Z

@mattstratton then just use -x to exclude a folder.

tas50 · 2016-11-23T23:03:16Z

We need to fix this in Foodcritic itself. The default exclude needs to include the test dir. It causes issues in Delivery and also for any user just running Foodcritic from the CLI.

charlesjohnson · 2016-12-31T00:33:24Z

chef -v
Chef Development Kit Version: 1.1.16
chef-client version: 12.17.44
delivery version: master (83358fb62c0f711c70ad5a81030a6cae4017f103)
berks version: 5.2.0
kitchen version: 1.14.2

GOOD NEWS: Foodcritic no longer fails due to the test directory.

BAD NEWS: Foodcritic now fails for a completely different reason, that is arguably Foodcritic's fault, not the cookbook's fault:

➜  chef generate cookbook baz
Generating cookbook baz
- Ensuring correct cookbook file content
- Committing cookbook files to git
- Ensuring delivery configuration
- Ensuring correct delivery build cookbook content
- Adding delivery configuration to feature branch
- Adding build cookbook to feature branch
- Merging delivery content feature branch to master

Your cookbook is ready. Type `cd baz` to enter it.

*snip*

➜  foodcritic -f correctness baz
FC064: Ensure issues_url is set in metadata: baz/metadata.rb:1
FC065: Ensure source_url is set in metadata: baz/metadata.rb:1

charlesjohnson · 2016-12-31T00:43:48Z

Should we keep looking at this here, or should we take it to Foodcritic/foodcritic#503 ?

qubitrenegade · 2017-01-03T18:54:52Z

➜ foodcritic -f correctness baz
FC064: Ensure issues_url is set in metadata: baz/metadata.rb:1
FC065: Ensure source_url is set in metadata: baz/metadata.rb:1

This to me is a perfectly reasonable reason for FoodCritic to fail. It would be nice if you could set that somehow (CLI flag and/or knife.rb config), however, I think this is a markedly different failure than failing because the fundamental layout of your cookbook dir structure is "incorrect".

nathenharvey · 2017-01-03T19:18:57Z

Neither FC064 nor FC065 are tagged with correctness.

Output from foodcritic is not an indication of failure, the exit code should be checked for that.

If you want to ensure both the output and the command are only showing results of testing for rules tagged with correctness you need to use foodcritic -f correctness -t correctness baz

nathenharvey · 2017-01-05T03:13:38Z

I believe this issue can now be closed. What do you say @mattstratton?

cheeseplus · 2017-01-26T02:15:39Z

Foodcritic now excludes test/ dirs by default as of Foodcritic/foodcritic@aa69442

tas50 · 2017-01-26T02:59:19Z

Expect a release of Foodcritic within the next few days, which will ship in next months Chef-DK

vinyar · 2017-04-24T22:36:34Z

Still an issue in ChefDK 1.3.43

$ chef generate cookbook trash                                                                     [ruby-2.3.1p112]
Generating cookbook trash
...
Your cookbook is ready. To setup the pipeline, type `cd trash`, then run `delivery init`
alexvinyar@seavinyar02 ~/Documents/git/cookbooks $ cd trash                                                                                         [ruby-2.3.1p112]
alexvinyar@seavinyar02 ~/Documents/git/cookbooks/trash (master)$ foodcritic . -f correctness                                                        [ruby-2.3.1p112]
Checking 2 files
x.
FC064: Ensure issues_url is set in metadata: ./metadata.rb:1
FC065: Ensure source_url is set in metadata: ./metadata.rb:1
FC067: Ensure at least one platform supported in metadata: ./metadata.rb:1
alexvinyar@seavinyar02 ~/Documents/git/cookbooks/trash (master)$ chef -v                                                                            [ruby-2.3.1p112]
Chef Development Kit Version: 1.3.43
chef-client version: 12.19.36
delivery version: master (dd319aa632c2f550c92a2172b9d1226478fea997)
berks version: 5.6.4
kitchen version: 1.16.0
alexvinyar@seavinyar02 ~/Documents/git/cookbooks/trash (master)$ chef gem list foodc                                                                [ruby-2.3.1p112]

*** LOCAL GEMS ***

foodcritic (10.2.2)

The interesting thing, is that with foodcritic 11.0.0 it actually gets a tiny bit worse as it's now failing on FC078:

$ foodcritic -V              
foodcritic 11.0.0

foodcritic . -f correctness                                                        [ruby-2.3.1p112]
Checking 2 files
x.
FC008: Generated cookbook metadata needs updating: ./metadata.rb:2
FC008: Generated cookbook metadata needs updating: ./metadata.rb:3
FC064: Ensure issues_url is set in metadata: ./metadata.rb:1
FC065: Ensure source_url is set in metadata: ./metadata.rb:1
FC067: Ensure at least one platform supported in metadata: ./metadata.rb:1
FC078: Ensure cookbook shared under an OSI-approved open source license: ./metadata.rb:1

charlesjohnson · 2017-04-25T22:14:45Z

At this point our own tooling is executing foodcritic with foodcritic . -f any -t "~supermarket".

Is there a strong need from customers to pass out of the box without setting the metadata correctly for Supermarket?

tas50 · 2017-04-25T23:14:19Z

So we're going to have a hard time with this going forward. Foodcritic sat idle for about 2 years, but I've added 7 rules in the last week. There's going to be a lot more rules coming, which will constantly add new requirements. Also keep in mind that and and out of the box cookbook SHOULD NOT pass. It's not a functional cookbook. It's just a generated skeleton that needs modification before it's ready.

charlesjohnson · 2017-04-25T23:47:38Z

We might need to find a compromise by which out-of-the-box cookbooks can pass foodcritic in an Automate Workflow context.

Because Automate uses the initial project skeleton commit to test out the pipeline, if there's a pipeline failure in the foodcritic phase, then that'll break the pipeline until the project passes.

Not a deal breaker, but I think it impacts the use cases that @vinyar sees.

@vinyar: Am I putting words in your mouth?

qubitrenegade · 2017-04-26T15:42:27Z

@charlesjohnson when this issue was first opened the issue was that foodcritic was generating erroneous errors based on changes to the default test framework (i.e.: the change over from ServerSpec to InSpec). The change in directory structure caused FoodCritic to incorrectly find that there was no README.md, etc. for the "cookbook" in the tests/ directory.

The errors posted by @vinyar are not erroneous however. These are errors that I would expect to receive until you fix them. These indicate there is something wrong with your cookbook. Your cookbook should have an issues and source URL, even if they're the same (they are in our environment for instance), your cookbook should declare platforms it supports, etc. As these errors are not erroneous and are reporting things that are wrong in the cookbook, I don't think these failures really fit in with the original bug report.

In my opinion, the fix for the original issue reported by @mattstratton an update to ChefDK, the fix for @vinyar problem is to update his metadata.

I would even argue further that these SHOULD cause automate/foodcritic to explode, if I'm collaborating with a team (pushing cookbooks through Automate or not) then it's even more important to tell people where you can find source, what platforms my cookbook supports, etc. Our standard for communicating that information is to put it in the Metadata.

(Having said that, it would be nice to be able to set these as part of the chef generate cookbook process either with a CLI flag or by setting attributes in your .chef/knife.rb or .chef/config.rb. but that's perhaps a topic for another thread)

charlesjohnson · 2017-04-26T16:36:32Z

@mattstratton is your issue resolved?

@vinyar What do you think of the discussion since your update?

mattstratton · 2017-04-26T17:13:39Z

From my perspective, my issue has been resolved.

cheeseplus · 2017-07-12T20:07:22Z

Initial issue is closed and the other issue is both by design and unrelated.

vinyar · 2017-07-14T22:02:46Z

@charlesjohnson the use-case you described is what I was after.
In the grand scheme of things, I could argue that my version of the bug mostly affects newer customers as it takes more gymnastics to get going, resulting in a very rocky early experience. Advanced users will all run custom cookbook generators anyway.

If the initial commit fails, the config.json will need to be modified. That puts onus on the user to re-enable testing with the very next commit, or in a less desireable case - at some point in the future.

burtlo pushed a commit to chef-training/site_config that referenced this issue Sep 26, 2016

This is a workaround to a Chef DK issue (chef-boneyard/chef-dk#1027)

48aec61

qubitrenegade mentioned this issue Sep 30, 2016

Chef generate cookbook creates tests in a different location than where kitchen-inspec looks for tests #964

Closed

cheeseplus closed this as completed Jul 12, 2017

chef-boneyard locked and limited conversation to collaborators Feb 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cookbook created by chef generate cookbook fails foodcritic due to test directory #1027

cookbook created by chef generate cookbook fails foodcritic due to test directory #1027

mattstratton commented Sep 25, 2016

qubitrenegade commented Sep 29, 2016

vinyar commented Oct 4, 2016

vinyar commented Oct 4, 2016

vinyar commented Oct 5, 2016 •

edited

Loading

mattstratton commented Oct 5, 2016

vinyar commented Oct 5, 2016

tas50 commented Nov 23, 2016

charlesjohnson commented Dec 31, 2016

charlesjohnson commented Dec 31, 2016

qubitrenegade commented Jan 3, 2017

nathenharvey commented Jan 3, 2017

nathenharvey commented Jan 5, 2017

cheeseplus commented Jan 26, 2017

tas50 commented Jan 26, 2017

vinyar commented Apr 24, 2017

charlesjohnson commented Apr 25, 2017

tas50 commented Apr 25, 2017

charlesjohnson commented Apr 25, 2017

qubitrenegade commented Apr 26, 2017

charlesjohnson commented Apr 26, 2017

mattstratton commented Apr 26, 2017

cheeseplus commented Jul 12, 2017

vinyar commented Jul 14, 2017

cookbook created by chef generate cookbook fails foodcritic due to test directory #1027

cookbook created by chef generate cookbook fails foodcritic due to test directory #1027

Comments

mattstratton commented Sep 25, 2016

Description

ChefDK Version

Platform Version

Replication Case

qubitrenegade commented Sep 29, 2016

vinyar commented Oct 4, 2016

vinyar commented Oct 4, 2016

vinyar commented Oct 5, 2016 • edited Loading

mattstratton commented Oct 5, 2016

vinyar commented Oct 5, 2016

tas50 commented Nov 23, 2016

charlesjohnson commented Dec 31, 2016

charlesjohnson commented Dec 31, 2016

qubitrenegade commented Jan 3, 2017

nathenharvey commented Jan 3, 2017

nathenharvey commented Jan 5, 2017

cheeseplus commented Jan 26, 2017

tas50 commented Jan 26, 2017

vinyar commented Apr 24, 2017

charlesjohnson commented Apr 25, 2017

tas50 commented Apr 25, 2017

charlesjohnson commented Apr 25, 2017

qubitrenegade commented Apr 26, 2017

charlesjohnson commented Apr 26, 2017

mattstratton commented Apr 26, 2017

cheeseplus commented Jul 12, 2017

vinyar commented Jul 14, 2017

vinyar commented Oct 5, 2016 •

edited

Loading