(MODULES-9428) show how simple_get_filter+composite namevars is broken

This change adds acceptance tests showing how enabling `simple_get_filter`
for a type with composite namevars will break when a user sets a custom
title for a resource and uses attributes to pass in the namevar values
instead:

```
let(:manifest) { 'composite_namevar { "sometitle": ensure => "present", package => "php", manager => "wibble" }' }

it { expect(@stdout_str).to match %r{Looking for \["sometitle"\]} }
```

This behaviour is misleading at best (as authors might expect `name` to
always match all namevars through a title pattern) and completely
debilitating at worst, when a custom title is passed to `delete` and
there is no indication which resource to delete.

Author: DavidS

spec/acceptance/composite_namevar_spec.rb

@@ -1,5 +1,6 @@
-require 'spec_helper'
 require 'open3'
+require 'spec_helper'
+require 'tempfile'
 
 RSpec.describe 'a type with composite namevars' do
   let(:common_args) { '--verbose --trace --debug --strict=error --modulepath spec/fixtures' }
@@ -8,6 +9,7 @@
     it 'is returns the values correctly' do
       stdout_str, status = Open3.capture2e("puppet resource #{common_args} composite_namevar")
       expect(stdout_str.strip).to match %r{^composite_namevar}
+      expect(stdout_str.strip).to match %r{Looking for \[\]}
       expect(status).to eq 0
     end
     it 'returns the required resource correctly' do
@@ -16,23 +18,27 @@
       expect(stdout_str.strip).to match %r{ensure\s*=> \'present\'}
       expect(stdout_str.strip).to match %r{package\s*=> \'php\'}
       expect(stdout_str.strip).to match %r{manager\s*=> \'yum\'}
+      expect(stdout_str.strip).to match %r{Looking for \[\]}
       expect(status.exitstatus).to eq 0
     end
-    it 'Throws error if title is not a matching title_pattern' do
-      stdout_str, status = Open3.capture2e("puppet resource #{common_args} composite_namevar php package=php manager=yum")
-      expect(stdout_str.strip).to match %r{No set of title patterns matched the title "php"}
+    it 'throws error if title is not a matching title_pattern' do
+      stdout_str, status = Open3.capture2e("puppet resource #{common_args} composite_namevar php123 package=php manager=yum")
+      expect(stdout_str.strip).to match %r{No set of title patterns matched the title "php123"}
+      expect(stdout_str.strip).not_to match %r{Looking for}
       expect(status.exitstatus).to eq 1
     end
     it 'returns the match if alternative title_pattern matches' do
       stdout_str, status = Open3.capture2e("puppet resource #{common_args} composite_namevar php/gem")
       expect(stdout_str.strip).to match %r{^composite_namevar \{ \'php/gem\'}
       expect(stdout_str.strip).to match %r{ensure\s*=> \'present\'}
+      expect(stdout_str.strip).to match %r{Looking for \["php/gem"\]}
       expect(status.exitstatus).to eq 0
     end
     it 'properly identifies an absent resource if only the title is provided' do
       stdout_str, status = Open3.capture2e("puppet resource #{common_args} composite_namevar php-wibble")
       expect(stdout_str.strip).to match %r{^composite_namevar \{ \'php-wibble\'}
       expect(stdout_str.strip).to match %r{ensure\s*=> \'absent\'}
+      expect(stdout_str.strip).to match %r{Looking for \["php-wibble"\]}
       expect(status.exitstatus).to eq 0
     end
     it 'creates a previously absent resource' do
@@ -41,6 +47,7 @@
       expect(stdout_str.strip).to match %r{ensure\s*=> \'present\'}
       expect(stdout_str.strip).to match %r{package\s*=> \'php\'}
       expect(stdout_str.strip).to match %r{manager\s*=> \'wibble\'}
+      expect(stdout_str.strip).to match %r{Looking for \["php-wibble"\]}
       expect(status.exitstatus).to eq 0
     end
     it 'will remove an existing resource' do
@@ -49,16 +56,16 @@
       expect(stdout_str.strip).to match %r{package\s*=> \'php\'}
       expect(stdout_str.strip).to match %r{manager\s*=> \'gem\'}
       expect(stdout_str.strip).to match %r{ensure\s*=> \'absent\'}
+      expect(stdout_str.strip).to match %r{Looking for \["php-gem"\]}
       expect(status.exitstatus).to eq 0
     end
   end
 
   describe 'using `puppet apply`' do
-    require 'tempfile'
-
     let(:common_args) { super() + ' --detailed-exitcodes' }
 
-    # run Open3.capture2e only once to get both output, and exitcode # rubocop:disable RSpec/InstanceVariable
+    # run Open3.capture2e only once to get both output, and exitcode
+    # rubocop:disable RSpec/InstanceVariable
     before(:each) do
       Tempfile.create('acceptance') do |f|
         f.write(manifest)
@@ -67,42 +74,82 @@
       end
     end
 
-    context 'when managing a present instance' do
-      let(:manifest) { 'composite_namevar { php-gem: }' }
+    context 'when matching title patterns' do
+      context 'when managing a present instance' do
+        let(:manifest) { 'composite_namevar { php-gem: }' }
 
-      it { expect(@stdout_str).to match %r{Current State: \{:title=>"php-gem", :package=>"php", :manager=>"gem", :ensure=>"present", :value=>"b"\}} }
-      it { expect(@status.exitstatus).to eq 0 }
-    end
+        it { expect(@stdout_str).to match %r{Current State: \{:title=>"php-gem", :package=>"php", :manager=>"gem", :ensure=>"present", :value=>"b"\}} }
+        it { expect(@stdout_str).to match %r{Looking for \["php-gem"\]} }
+        it { expect(@status.exitstatus).to eq 0 }
+      end
 
-    context 'when managing an absent instance' do
-      let(:manifest) { 'composite_namevar { php-wibble: ensure=>\'absent\' }' }
+      context 'when managing an absent instance' do
+        let(:manifest) { 'composite_namevar { php-wibble: ensure=>\'absent\' }' }
 
-      it { expect(@stdout_str).to match %r{Composite_namevar\[php-wibble\]: Nothing to manage: no ensure and the resource doesn't exist} }
-      it { expect(@status.exitstatus).to eq 0 }
-    end
+        it { expect(@stdout_str).to match %r{Composite_namevar\[php-wibble\]: Nothing to manage: no ensure and the resource doesn't exist} }
+        it { expect(@stdout_str).to match %r{Looking for \["php-wibble"\]} }
+        it { expect(@status.exitstatus).to eq 0 }
+      end
 
-    context 'when creating a previously absent instance' do
-      let(:manifest) { 'composite_namevar { php-wibble: ensure=>\'present\' }' }
+      context 'when creating a previously absent instance' do
+        let(:manifest) { 'composite_namevar { php-wibble: ensure=>\'present\' }' }
 
-      it { expect(@stdout_str).to match %r{Composite_namevar\[php-wibble\]/ensure: defined 'ensure' as 'present'} }
-      it { expect(@status.exitstatus).to eq 2 }
-    end
+        it { expect(@stdout_str).to match %r{Composite_namevar\[php-wibble\]/ensure: defined 'ensure' as 'present'} }
+        it { expect(@stdout_str).to match %r{Looking for \["php-wibble"\]} }
+        it { expect(@status.exitstatus).to eq 2 }
+      end
 
-    context 'when removing a previously present instance' do
-      let(:manifest) { 'composite_namevar { php-yum: ensure=>\'absent\' }' }
+      context 'when removing a previously present instance' do
+        let(:manifest) { 'composite_namevar { php-yum: ensure=>\'absent\' }' }
 
-      it { expect(@stdout_str).to match %r{Composite_namevar\[php-yum\]/ensure: undefined 'ensure' from 'present'} }
-      it { expect(@status.exitstatus).to eq 2 }
-    end
+        it { expect(@stdout_str).to match %r{Composite_namevar\[php-yum\]/ensure: undefined 'ensure' from 'present'} }
+        it { expect(@stdout_str).to match %r{Looking for \["php-yum"\]} }
+        it { expect(@status.exitstatus).to eq 2 }
+      end
 
-    context 'when modifying an existing resource through an alternative title_pattern' do
-      let(:manifest) { 'composite_namevar { \'php/gem\': value=>\'c\' }' }
+      context 'when modifying an existing resource through an alternative title_pattern' do
+        let(:manifest) { 'composite_namevar { \'php/gem\': value=>\'c\' }' }
 
-      it { expect(@stdout_str).to match %r{Current State: \{:title=>"php-gem", :package=>"php", :manager=>"gem", :ensure=>"present", :value=>"b"\}} }
-      it { expect(@stdout_str).to match %r{Target State: \{:package=>"php", :manager=>"gem", :value=>"c", :ensure=>"present"\}} }
-      it { expect(@status.exitstatus).to eq 2 }
+        it { expect(@stdout_str).to match %r{Current State: \{:title=>"php-gem", :package=>"php", :manager=>"gem", :ensure=>"present", :value=>"b"\}} }
+        it { expect(@stdout_str).to match %r{Target State: \{:package=>"php", :manager=>"gem", :value=>"c", :ensure=>"present"\}} }
+        it { expect(@stdout_str).to match %r{Looking for \["php/gem"\]} }
+        it { expect(@status.exitstatus).to eq 2 }
+      end
     end
 
+    context 'when using attributes' do
+      context 'when managing a present instance' do
+        let(:manifest) { 'composite_namevar { "sometitle": package => "php", manager => "gem" }' }
+
+        it { expect(@stdout_str).to match %r{Current State: \{:title=>"php-gem", :package=>"php", :manager=>"gem", :ensure=>"present", :value=>"b"\}} }
+        it { expect(@stdout_str).to match %r{Looking for \["sometitle"\]} }
+        it { expect(@status.exitstatus).to eq 0 }
+      end
+
+      context 'when managing an absent instance' do
+        let(:manifest) { 'composite_namevar { "sometitle": ensure => "absent", package => "php", manager => "wibble" }' }
+
+        it { expect(@stdout_str).to match %r{Composite_namevar\[sometitle\]: Nothing to manage: no ensure and the resource doesn't exist} }
+        it { expect(@stdout_str).to match %r{Looking for \["sometitle"\]} }
+        it { expect(@status.exitstatus).to eq 0 }
+      end
+
+      context 'when creating a previously absent instance' do
+        let(:manifest) { 'composite_namevar { "sometitle": ensure => "present", package => "php", manager => "wibble" }' }
+
+        it { expect(@stdout_str).to match %r{Composite_namevar\[sometitle\]/ensure: defined 'ensure' as 'present'} }
+        it { expect(@stdout_str).to match %r{Looking for \["sometitle"\]} }
+        it { expect(@status.exitstatus).to eq 2 }
+      end
+
+      context 'when removing a previously present instance' do
+        let(:manifest) { 'composite_namevar { "sometitle": ensure => "absent", package => "php", manager => "yum" }' }
+
+        it { expect(@stdout_str).to match %r{Composite_namevar\[sometitle\]/ensure: undefined 'ensure' from 'present'} }
+        it { expect(@stdout_str).to match %r{Looking for \["sometitle"\]} }
+        it { expect(@status.exitstatus).to eq 2 }
+      end
+    end
     # rubocop:enable RSpec/InstanceVariable
   end
 end

spec/fixtures/test_module/lib/puppet/provider/composite_namevar/composite_namevar.rb

@@ -14,7 +14,8 @@ def initialize
     ]
   end
 
-  def get(_context)
+  def get(context, names = nil)
+    context.notice("Looking for #{names.inspect}")
     @current_values
   end
 

spec/fixtures/test_module/lib/puppet/type/composite_namevar.rb

@@ -5,6 +5,7 @@
   docs: <<-DOC,
     This type provides Puppet with the capabilities to manage ...
   DOC
+  features: [ 'simple_get_filter' ],
   title_patterns: [
     {
       pattern: %r{^(?<package>.*[^-])-(?<manager>.*)$},
@@ -14,6 +15,10 @@
       pattern: %r{^(?<package>.*[^/])/(?<manager>.*)$},
       desc: 'Where the package and the manager are provided with a forward slash separator',
     },
+    {
+      pattern: %r{^(?<package>[a-z]*[^/])$},
+      desc: 'Fallback pattern where only the package is provided',
+    },
   ],
   attributes:   {
     ensure:      {