so you want to contribute to puppet? a getting started guide

So You Want to Contribute to Puppet? A Getting Started GuideRuth Linehan & Hailee KenneyEngineering | Puppet Labs @ruthlinehan, @haileekenney

Saturday, August 24, 13

puppetconf.com #puppetconf

What Do We Want You to Take Away?• Skills

• Tools

• Confidence



Why Contribute?

• It’s valuable for you

• It’s valuable for us

• It’s valuable for the community



Non-Code Ways to Contribute



Projects You Can Contribute To• Puppet

• Facter

• Hiera

• PuppetDB

• Modules (stdlib, ntp, mysql, apache...)



How Have We Made Things Easier?• Dedicated Community Dev Team

• Simplified Process



Workflow Overview1. Redmine

2. Getting code from GitHub

3. Write some code!

4. Test your code!

5. Signing the CLA

6. Submitting code via GitHub

7. Our Response



Redmine (projects.puppetlabs.com)



Finding/Creating an Issue



Forking the Repository



Cloning Your Fork$ git clone https://github.com/rlinehan/puppetlabs-stdlib remote: Counting objects: 4097, done. remote: Compressing objects: 100% (2316/2316), done. remote: Total 4097 (delta 1600), reused 3730 (delta 1279) Receiving objects: 100% (4097/4097), 716.80 KiB | 156 KiB/s, done. Resolving deltas: 100% (1600/1600), done.

$ cd puppetlabs-stdlib

puppetlabs-stdlib (git:master:2a78cbf) $ lsCHANGELOG CONTRIBUTING.md Gemfile LICENSE

Modulefile README.markdown README_DEVELOPER.markdown README_SPECS.markdown

RELEASE_PROCESS.markdown Rakefile lib lstext


https://github.com/rlinehan/puppetlabs-stdlib

https://github.com/rlinehan/puppetlabs-stdlib


Creating Topic Branch

$ git checkout -b ticket/master/12345_fix_facter_issueSwitched to a new branch 'ticket/master/12345_fix_facter_issue'



Setting Up Your Environment

• Gems

• Bundler

• Ruby Version

• 1.8.7

• 1.9.3

• 2.0.0

localhost:~/Development/facter $ bundle installResolving dependencies... Using rake (10.1.0) Using coderay (1.0.9) Using diff-lcs (1.1.3) Using facter (1.7.2) from source at . Using json (1.8.0) Using metaclass (0.0.1) Using method_source (0.8.2) Using mocha (0.10.5) Using slop (3.4.6) Using pry (0.9.12.2) Using rspec-core (2.11.1) Using rspec-expectations (2.11.3) Using rspec-mocks (2.11.3) Using rspec (2.11.0) Using rspec-puppet (0.1.6) Using puppetlabs_spec_helper (0.4.1.3) Using redcarpet (3.0.0) Using watchr (0.7) Using yard (0.8.7) Using bundler (1.3.5) Your bundle is complete! Use `bundle show [gemname]` to see where a bundled gem is installed.



Ruby Gems# C Ruby (MRI) or Rubinius, but NOT Windows platforms :ruby do gem 'watchr', :group => :development gem 'pry', :group => :development gem 'yard', :group => :development gem 'redcarpet', :group => :development end

group :development, :test do gem 'rake' gem 'facter', ">= 1.0.0", :path => File.expand_path("..", __FILE__) gem 'rspec', "~> 2.11.0" gem 'mocha', "~> 0.10.5" gem 'json', "~> 1.7" gem 'puppetlabs_spec_helper' end

platform :mswin, :mingw do gem "win32-api", "~> 1.4.8" gem "win32-dir", "~> 0.3.7" gem "windows-api", "~> 0.4.1" gem "windows-pr", "~> 1.2.1" gem "win32console", "~> 1.3.2" end puppet/Gemfile



Why Do We Need Tests?

[Photograph of what happens when you forget the trailing semicolon on a C++ class declaration.]

http://theprofoundprogrammer.com/





Running Tests$ bundle exec rspec spec --color............................................................................................................................

Finished in 0.09627 seconds 124 examples, 0 failures

localhost:~/Development/hiera $ bundle exec rspec spec/unit/hiera_spec.rb -fd -cHiera #logger= loads the given logger falls back to the Console logger when the logger could not be loaded falls back to the Console logger when the logger class could not be found #warn delegates to the configured logger #debug delegates to the configured logger #initialize uses a default config file when none is provided passes the supplied config to the config class loads all backends on start #lookup delegates to the Backend#lookup method

Finished in 0.00542 seconds 9 examples, 0 failures



Running Tests$ bundle exec rspec --color spec......................................................................................................F*..................

Pending: Hiera#warn delegates to the configured logger # No reason given # ./spec/unit/hiera_spec.rb:35

Failures:

1) Hiera#logger= falls back to the Console logger when the logger class could not be found Failure/Error: Hiera.expects(:require).with("hiera/no_constant_loggers") Mocha::ExpectationError: not all expectations were satisfied unsatisfied expectations: - expected exactly once, not yet invoked: Hiera.require('hiera/no_constant_loggers') satisfied expectations: - expected exactly once, invoked once: Hiera.warn(any_parameters) # ./spec/unit/hiera_spec.rb:26:in `block (3 levels) in <top (required)>'

Finished in 0.10318 seconds 122 examples, 1 failure, 1 pending

Failed examples:

rspec ./spec/unit/hiera_spec.rb:24 # Hiera#logger= falls back to the Console logger when the logger class could not be found



Fun With RSpec Formatting

gem 'nyan-cat-formatter'

$ bundle exec rspec spec/unit/hiera_spec.rb --format NyanCatFormatter



How to Write Testsdef self.warn(msg) if Facter.debugging? and msg and not msg.empty? msg = [msg] unless msg.respond_to? :each msg.each { |line| Kernel.warn line } end end

describe "when warning" do it "should warn if debugging is enabled" do Facter.debugging(true) Kernel.stubs(:warn) Kernel.expects(:warn).with('foo') Facter.warn('foo') end

it "should not warn if debugging is enabled but nil is passed" do Facter.debugging(true) Kernel.stubs(:warn) Kernel.expects(:warn).never Facter.warn(nil) end

it "should not warn if debugging is enabled but an empty string is passed" do Facter.debugging(true) Kernel.stubs(:warn) Kernel.expects(:warn).never Facter.warn('') end

code: facter/lib/facter.rbtests: facter/spec/unit/facter_spec.rb



How to Write Testsdef self.warn(msg) if Facter.debugging? and msg and not msg.empty? msg = [msg] unless msg.respond_to? :each msg.each { |line| Kernel.warn line } end end

it "should not warn if debugging is disabled" do Facter.debugging(false) Kernel.stubs(:warn) Kernel.expects(:warn).never Facter.warn('foo') end

it "should warn for any given element for an array if debugging is enabled" do Facter.debugging(true) Kernel.stubs(:warn) Kernel.expects(:warn).with('foo') Kernel.expects(:warn).with('bar') Facter.warn( ['foo','bar']) end end

code: facter/lib/facter.rbtests: facter/spec/unit/facter_spec.rb



How NOT to Write Testsdescribe "when applying changes" do [false, true].each do |noop_mode|; describe (noop_mode ? "in noop mode" : "in normal mode") do [nil, @mode_750].each do |machine_state|; describe (machine_state ? "with a file initially present" : "with no file initially present") do [nil, @mode_750, @mode_755].each do |yaml_mode| [nil, :file, :absent].each do |yaml_ensure|; describe "with mode=#{yaml_mode.inspect} and ensure=#{yaml_ensure.inspect} stored in state.yml" do [false, true].each do |auditing_ensure| [false, true].each do |auditing_mode| auditing = [] auditing.push(:mode) if auditing_mode auditing.push(:ensure) if auditing_ensure [nil, :file, :absent].each do |ensure_property| # what we set "ensure" to in the manifest [nil, @mode_750, @mode_755].each do |mode_property| # what we set "mode" to in the manifest manifest_settings = {} manifest_settings[:audit] = auditing if !auditing.empty? manifest_settings[:ensure] = ensure_property if ensure_property manifest_settings[:mode] = mode_property if mode_property describe "with manifest settings #{manifest_settings.inspect}" do; it "should behave properly" do # Set up preconditions test_file = tmpfile('foo') if machine_state File.open(test_file, 'w', machine_state.to_i(8)).close end

puppet/spec/unit/transaction/resource_harness_spec.rb



How NOT to Write Tests

# Check the :synced field on state.yml synced_should_be_set = !noop_mode && status.changed (@harness.cached(resource, :synced) != nil).should == synced_should_be_set end; end end end end end end; end end end; end end; end

puppet/spec/unit/transaction/resource_harness_spec.rb



git status# On branch ticket/master/12345_fix_facter_issue

# Changes not staged for commit: # (use "git add <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # # modified: spec/unit/kernel_spec.rb # # Untracked files: # (use "git add <file>..." to include in what will be committed) # # spec/unit/new_test_file.rb no changes added to commit (use "git add" and/or "git commit -a")



git status

# On branch ticket/master/12345_fix_facter_issue # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # modified: spec/unit/kernel_spec.rb # new file: spec/unit/new_test_file.rb # # Changes not staged for commit: # (use "git add <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # # modified: spec/unit/new_test_file.rb #



git diffdiff --git a/spec/unit/kernel_spec.rb b/spec/unit/kernel_spec.rb index 37d3431..053d651 100644 --- a/spec/unit/kernel_spec.rb +++ b/spec/unit/kernel_spec.rb @@ -5,6 +5,8 @@ require 'spec_helper' describe "Kernel fact" do include FacterSpec::ConfigHelper + # Adding a comment to explain something about this test + describe "on Windows" do it "should return the kernel as 'windows'" do given_a_configuration_of(:is_windows => true, :data_dir => "data_dir")



Committing as You Go



Good Commit Messages(#99999) Make the example in CONTRIBUTING imperative and concrete

Without this patch applied the example commit message in the CONTRIBUTING document is not a concrete example. This is a problem because the contributor is left to imagine what the commit message should look like based on a description rather than an example. This patch fixes the problem by making the example concrete and imperative.

The first line is a real life imperative statement with a ticket number from our issue tracker. The body describes the behavior without the patch, why this is a problem, and how the patch fixes the problem when applied.



Cleaning Up Commits

facter (git:test_rebasing*:8ca7ec5) $ git lg

* 8ca7ec5 Add second test example 2 minutes ago Ruth Linehan (HEAD, test_rebasing)* 4a5e319 whitespace errors are stupid 2 minutes ago Ruth Linehan* 02004f4 kernel spec test is wrong!!!!!!1111 4 minutes ago Ruth Linehan* d1e8046 Merge branch 'erjohnso-gce' closes #523 29 hours ago Jeff McCune (origin/master, origin/HEAD, master)|\| * 113ddc2 (#22233) Add license information for GCE facts 5 days ago Eric Johnson| * d7c863e (#22233) Add facts from Google Compute Engine metadata service 5 days ago Eric Johnson

$ git rebase -i d1e8046



Interactive Rebase

# This is a combination of 2 commits. # The first commit's message is:

Update kernel spec

Previously the kernel spec test failed because a test value was incorrect. This commit fixes it by correcting the value.

# This is the 2nd commit message:

whitespace errors are stupid

# Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # Not currently on any branch. # You are currently editing a commit during a rebase. # # Changes to be committed: # (use "git reset HEAD^1 <file>..." to unstage) # # modified: spec/unit/kernel_spec.rb

reword 8ca7ec5 kernel spec test is wrong!!!!!!1111 squash 16ef267 whitespace errors are stupid pick c34ac79 Add second test example

# Rebase d1e8046..c34ac79 onto d1e8046 # # Commands: # p, pick = use commit # r, reword = use commit, but edit the commit message # e, edit = use commit, but stop for amending # s, squash = use commit, but meld into previous commit # f, fixup = like "squash", but discard this commit's log message # x, exec = run command (the rest of the line) using shell

commit e36eda6edfebf25759146fede28dd2554ed66f6d Author: Ruth Linehan <[email protected]> Date: Wed Aug 21 22:32:00 2013 -0700

Update kernel spec Previously the kernel spec test failed because a test value was incorrect. This commit fixes it by correcting the value. Also fix a whitespace error.


mailto:[email protected]

mailto:[email protected]


Signing the CLA



Submitting a Pull Request



What Now?



Need Help?• ask.puppetlabs.com

• IRC Channels

• #puppet-dev

• #puppet

• Google groups

• puppet-users

• puppet-dev



Resources

http://dft.ba/-contribute-to-puppet




Thank YouRuth Linehan & Hailee KenneyEngineering | Puppet Labs @ruthlinehan, @haileekenney

Collaborate. Automate. Ship.


Follow us on Twitter @puppetlabs

youtube.com/puppetlabsinc

slideshare.net/puppetlabs

Collaborate. Automate. Ship.


so you want to contribute to puppet? a getting started guide

Technology

hailee kenney

puppet labs

write tests

pull request

logger class

console logger

rb tests

git add