Merge pull request #820 from alexharv074/maint_clarify_docs

(maint) Clarify docs and add new tests

Author: tphoney

README.md

@@ -117,29 +117,55 @@ In the example above, `match` looks for a line beginning with 'export' followed
 
 Match Example:
 
-    file_line { 'bashrc_proxy':
-      ensure             => present,
-      path               => '/etc/bashrc',
-      line               => 'export HTTP_PROXY=http://squid.puppetlabs.vm:3128',
-      match              => '^export\ HTTP_PROXY\=',
-      append_on_no_match => false,
-    }
+```puppet
+file_line { 'bashrc_proxy':
+  ensure             => present,
+  path               => '/etc/bashrc',
+  line               => 'export HTTP_PROXY=http://squid.puppetlabs.vm:3128',
+  match              => '^export\ HTTP_PROXY\=',
+  append_on_no_match => false,
+}
+```
 
 In this code example, `match` looks for a line beginning with export followed by HTTP_PROXY and replaces it with the value in line. If a match is not found, then no changes are made to the file.
 
-Match Example with `ensure => absent`:
+Examples With `ensure => absent`:
+
+This type has two behaviors when `ensure => absent` is set.
+
+One possibility is to set `match => ...` and `match_for_absence => true`,
+as in the following example:
 
 ```puppet
 file_line { 'bashrc_proxy':
   ensure            => absent,
   path              => '/etc/bashrc',
-  line              => 'export HTTP_PROXY=http://squid.puppetlabs.vm:3128',
   match             => '^export\ HTTP_PROXY\=',
   match_for_absence => true,
 }
 ```
 
-In the example above, `match` looks for a line beginning with 'export' followed by 'HTTP_PROXY' and deletes it. If multiple lines match, an error is raised, unless the `multiple => true` parameter is set.
+In this code example match will look for a line beginning with export
+followed by HTTP_PROXY and delete it.  If multiple lines match, an
+error will be raised unless the `multiple => true` parameter is set.
+
+Note that the `line => ...` parameter would be accepted *but ignored* in
+the above example.
+
+The second way of using `ensure => absent` is to specify a `line => ...`,
+and no match:
+
+```puppet
+file_line { 'bashrc_proxy':
+  ensure => absent,
+  path   => '/etc/bashrc',
+  line   => 'export HTTP_PROXY=http://squid.puppetlabs.vm:3128',
+}
+```
+
+Note that when ensuring lines are absent this way, the default behavior
+this time is to always remove all lines matching, and this behavior
+can't be disabled.
 
 Encoding example:
 

lib/puppet/type/file_line.rb

@@ -34,12 +34,16 @@
     In this code example match will look for a line beginning with export
     followed by HTTP_PROXY and replace it with the value in line.
 
-    Match Example With `ensure => absent`:
+    Examples With `ensure => absent`:
+
+    This type has two behaviors when `ensure => absent` is set.
+
+    One possibility is to set `match => ...` and `match_for_absence => true`,
+    as in the following example:
 
         file_line { 'bashrc_proxy':
           ensure            => absent,
           path              => '/etc/bashrc',
-          line              => 'export HTTP_PROXY=http://squid.puppetlabs.vm:3128',
           match             => '^export\ HTTP_PROXY\=',
           match_for_absence => true,
         }
@@ -48,6 +52,22 @@
     followed by HTTP_PROXY and delete it.  If multiple lines match, an
     error will be raised unless the `multiple => true` parameter is set.
 
+    Note that the `line => ...` parameter would be accepted BUT IGNORED in
+    the above example.
+
+    The second way of using `ensure => absent` is to specify a `line => ...`,
+    and no match:
+
+        file_line { 'bashrc_proxy':
+          ensure => absent,
+          path   => '/etc/bashrc',
+          line   => 'export HTTP_PROXY=http://squid.puppetlabs.vm:3128',
+        }
+
+    Note that when ensuring lines are absent this way, the default behavior
+    this time is to always remove all lines matching, and this behavior
+    can't be disabled.
+
     Encoding example:
 
         file_line { "XScreenSaver":

spec/unit/puppet/provider/file_line/ruby_spec.rb

@@ -337,7 +337,7 @@
     end
   end
 
-  context "when removing" do
+  context "when removing with a line" do
     before :each do
       # TODO: these should be ported over to use the PuppetLabs spec_helper
       #  file fixtures once the following pull request has been merged:
@@ -378,6 +378,23 @@
       @provider.destroy
       expect(File.read(@tmpfile)).to eql("foo1\nfoo2\n")
     end
+
+    it 'example in the docs' do
+      @resource = Puppet::Type::File_line.new(
+        {
+          :name   => 'bashrc_proxy',
+          :ensure => 'absent',
+          :path   => @tmpfile,
+          :line   => 'export HTTP_PROXY=http://squid.puppetlabs.vm:3128',
+        }
+      )
+      @provider = provider_class.new(@resource)
+      File.open(@tmpfile, 'w') do |fh|
+        fh.write("foo1\nfoo2\nexport HTTP_PROXY=http://squid.puppetlabs.vm:3128\nfoo4\n")
+      end
+      @provider.destroy
+      expect(File.read(@tmpfile)).to eql("foo1\nfoo2\nfoo4\n")
+    end
   end
 
   context "when removing with a match" do
@@ -416,6 +433,41 @@
       expect(File.read(@tmpfile)).to eql("foo1\nfoo2")
     end
 
+    it 'the line parameter is actually not used at all but is silently ignored if here' do
+      @resource = Puppet::Type::File_line.new(
+        {
+          :name              => 'foo',
+          :path              => @tmpfile,
+          :line              => 'supercalifragilisticexpialidocious',
+          :ensure            => 'absent',
+          :match             => 'o$',
+          :match_for_absence => true,
+        }
+      )
+      File.open(@tmpfile, 'w') do |fh|
+        fh.write("foo1\nfoo\nfoo2")
+      end
+      @provider.destroy
+      expect(File.read(@tmpfile)).to eql("foo1\nfoo2")
+    end
+
+    it 'and may not be here and does not need to be here' do
+      @resource = Puppet::Type::File_line.new(
+        {
+          :name              => 'foo',
+          :path              => @tmpfile,
+          :ensure            => 'absent',
+          :match             => 'o$',
+          :match_for_absence => true,
+        }
+      )
+      File.open(@tmpfile, 'w') do |fh|
+        fh.write("foo1\nfoo\nfoo2")
+      end
+      @provider.destroy
+      expect(File.read(@tmpfile)).to eql("foo1\nfoo2")
+    end
+
     it 'should raise an error if more than one line matches' do
       File.open(@tmpfile, 'w') do |fh|
         fh.write("foo1\nfoo\nfoo2\nfoo\nfoo")
@@ -480,6 +532,41 @@
       expect(File.read(@tmpfile)).to eql("foo1\nfoo\n")
     end
 
-  end
+    it 'example in the docs' do
+      @resource = Puppet::Type::File_line.new(
+        {
+          :name              => 'bashrc_proxy',
+          :ensure            => 'absent',
+          :path              => @tmpfile,
+          :line              => 'export HTTP_PROXY=http://squid.puppetlabs.vm:3128',
+          :match             => '^export\ HTTP_PROXY\=',
+          :match_for_absence => true,
+        }
+      )
+      @provider = provider_class.new(@resource)
+      File.open(@tmpfile, 'w') do |fh|
+        fh.write("foo1\nfoo2\nexport HTTP_PROXY=foo\nfoo4\n")
+      end
+      @provider.destroy
+      expect(File.read(@tmpfile)).to eql("foo1\nfoo2\nfoo4\n")
+    end
 
+    it 'example in the docs showing line is redundant' do
+      @resource = Puppet::Type::File_line.new(
+        {
+          :name              => 'bashrc_proxy',
+          :ensure            => 'absent',
+          :path              => @tmpfile,
+          :match             => '^export\ HTTP_PROXY\=',
+          :match_for_absence => true,
+        }
+      )
+      @provider = provider_class.new(@resource)
+      File.open(@tmpfile, 'w') do |fh|
+        fh.write("foo1\nfoo2\nexport HTTP_PROXY=foo\nfoo4\n")
+      end
+      @provider.destroy
+      expect(File.read(@tmpfile)).to eql("foo1\nfoo2\nfoo4\n")
+    end
+  end
 end

spec/unit/puppet/type/file_line_spec.rb

@@ -78,6 +78,9 @@
   it 'should not require that a line is specified when matching for absence' do
     expect { Puppet::Type.type(:file_line).new(:name => 'foo', :path => tmp_path, :ensure => :absent, :match_for_absence => :true, :match => 'match') }.not_to raise_error
   end
+  it 'although if a line is specified anyway when matching for absence it still works and the line is silently ignored' do
+    expect { Puppet::Type.type(:file_line).new(:name => 'foo', :path => tmp_path, :line => 'i_am_irrelevant', :ensure => :absent, :match_for_absence => :true, :match => 'match') }.not_to raise_error
+  end
   it 'should require that a file is specified' do
     expect { Puppet::Type.type(:file_line).new(:name => 'foo', :line => 'path') }.to raise_error(Puppet::Error, /path is a required attribute/)
   end