(DO NOT MERGE) (MODULES-3391) Run all unit tests with rake test, address test failure
#32
Conversation
This commit extends the existing rspec test matcher to encompass all tests under the spec directory - or, more specifically, the exact matcher that rspec's built-in Rake::Task supplies. This means it actually includes more directories than strictly exist under spec, but this perhaps is a bit of future proofing in case we ultimately add more subdirectories, e.g., 'defines'. Signed-off-by: Moses Mendoza <mendoza.moses@gmail.com>
Prior to this commit a test in the config_spec was failing with:
Mocha::ExpectationError:
unexpected invocation: #<AnyInstance:Win32::Registry>.[]('Domain')
satisfied expectations:
- allowed any number of times, invoked 4 times:
#<AnyInstance:Win32::Registry>.[]('ChocolateyInstall')
- allowed any number of times, not yet invoked:
#<Puppet::Util::Feature:0x45b5be0>.root?(any_parameters)
This is because a facter fact was making a call to Win32::Registry.[] which
hadn't been stubbed (two, in fact). This commit adds these stubs to the
spec_helper in addition to the existing one. This commit makes test pass, but
may be more brittle than a more generic stub that doesn't specify the
parameter. This commit assumes the specificity is of higher value.
Signed-off-by: Moses Mendoza <mendoza.moses@gmail.com>
| @@ -17,7 +17,7 @@ task :default => [:test] | |||
|
|
|||
| desc 'Run RSpec' | |||
| RSpec::Core::RakeTask.new(:test) do |t| | |||
| t.pattern = 'spec/{unit}/**/*.rb' | |||
| t.pattern = 'spec/{classes,defines,unit,functions,hosts,integration}/**/*_spec.rb' | |||
There was a problem hiding this comment.
You need to do a rake spec_prep prior to rake test in a clean repo. Not sure how this is expressed in the rakefile or doco.
There was a problem hiding this comment.
@glennsarti that's a great catch. I assumed that JJB was doing that, but looking closer at the spec runs it appears their restricted directory match was masking the fact they don't actually do the prep. This needs to be updated so that rake test => rake spec_prep
There was a problem hiding this comment.
@glennsarti a better option may be to simply not use this rake test task and instead use the rake spec task defined by puppetlabs_spec_helper, e.g., https://github.com/puppetlabs/puppetlabs_spec_helper/blob/master/lib/puppetlabs_spec_helper/rake_tasks.rb#L320-L325. This declares the appropriate dependencies already, not sure we need to duplicate. Thoughts?
There was a problem hiding this comment.
I was going to ask if this was necessary (the Rakefile), but went and looked at other modules Rakefiles and determined we were doing different things with different modules. Probably just a neglect thing, need to figure out the best thing and do that.
There was a problem hiding this comment.
@ferventcoder understood. After looking a bit closer and discussing with @ThoughtCrhyme and @glennsarti I think we can remove this rake::test task entirely and defer to rake::spec. This will require an associated PR to JJB to ensure our Spec Tests phase of the pipeline runs the right thing. Will link when that is up.
|
FYI associated unit test failures are attempts to install json_pure 2.0.2 on ruby 1.9.3 - see MODULES-3640 |
|
Appveyor/sync.yml is still using the older syntax of |
MosesMendoza
changed the title
rake test, address test failure
MosesMendoza
changed the title
rake test, address test failurerake test, address test failure
MosesMendoza
changed the title
rake test, address test failurerake test, address test failure
|
Closing. Will re-open a separate PR removing |
|
Superseded by #35 |
…e_laptop Updating site.pp for to include brew/bitbar
This PR extends the existing rspec test matcher in the
rake testtask to encompass all tests under the spec directory - or, more specifically, the exact matcher that rspec's built-inRake::Tasksupplies, in order to give us full coverage with this task. This means it actually includes more directories than strictly exist underspec, but this perhaps is a bit of future proofing in case we ultimately add more subdirectories, e.g.,defines. However, I'm happy to reduce this to the explicit set of directories that exist underspecif we think that's best.This PR also resolves a failing test:
This is because facter fact(s) were making a call to
Win32::Registry.[]which hadn't been stubbed (two, in fact). This commit adds these stubs to the spec_helper in addition to the existing one. This makes test pass, but may be more brittle than a more generic stub that doesn't specify the parameter. I assume we prefer higher specificity, but I'm happy to revise to a single line with justWin32::Registry.any_instance.stubs(:[])if preferred.