(SIMP-3430) Create Centralized spec_helper.rb #2

jeannegreulich · 2020-01-24T21:20:11Z

- created baseline for spec_helper.rb
  to do basc RSPEC configuration for SIMP module unit tests.
- added some basic methods that were also in spec_helpers
- added compliance markup methods for use in compliance
  markup unit tests.

SIMP-3430 #comment added simp-spec-helpers

- got spec_helpers.rb from pupmod-simp-pupmod - moved normalize function from pupmod tests to spec_helper - create global_spec_helper(dir, modulename) function to call from spec_helper.rb.

- created baseline for spec_helper.rb to do basc RSPEC configuration for SIMP module unit tests. - added some basic methods that were also in spec_helpers - added compliance markup methods for use in compliance markup unit tests. - note that you need compliance_markup in the fixtures file for these helpers to work. SIMP-3430 #comment added simp-spec-helpers

op-ct

Thanks for this patch--It's about time we returned to the the idea of centralizing our spec helper logic into a single gem.

Some concerns:

A monolithic global_spec_helper method may prescribe too much common functionality without providing individual modules' hooks to alter it (for instance, the current implementation makes default_hiera_config the only possible hiera config).
Quite a few line-item comments address things that are probably copypasta residue, like really old spec helper practices (like, last relevant in 2015 old) and updates to Hiera 5.
Now that we've centralized all this logic, the gem should probably have spec tests for it.
We'll need to take care that it doesn't introduce circular gem dependencies, though (IIRC, our previous attempt would have).

op-ct · 2020-02-04T20:44:39Z

.travis.yml

+rvm:
+  - 2.4.5
+env:
+  - SIMP_SKIP_NON_SIMPOS_TESTS=1


Is SIMP_SKIP_NON_SIMPOS_TESTS used by anything? It's not referenced in any other code in the repository.

op-ct · 2020-02-04T20:51:18Z

lib/simp/spec_helpers/helpers.rb

+      }
+
+      c.mock_framework = :rspec
+      c.mock_with  :mocha


Mock with :rspec, not :mocha.

most of them mok with mocha. You can overwrite it in the spec_helpers_local (example in README.) When we have moved most of our stuff over, we can change the default to rspec.

Most of our modules are configured for mocha as a matter of copy/pasta, but they don't actually mock anything—they run just as fine when configured for :rspec (I just tested this on a simp-core + rake deps:checkout, setting up a big bash loop over the simp- modules to make the changes to spec/spec_helper.rb and running spec tests for each one).

My concern is that there are problems with mocking in mocha¹ compared to rspec-mocks, and most of our modules aren't very invested yet. So I'd flip it around—we should encourage :rspec by default, and allow the modules who really can't and overwrite the spec_helpers_local for the modules that just can't switch just yet.

op-ct · 2020-02-04T21:05:57Z

lib/simp/spec_helpers/helpers.rb

+    EOM
+
+    if not File.directory?(File.join(fixture_path,'hieradata')) then
+      FileUtils.mkdir_p(File.join(fixture_path,'hieradata'))


This looks like legacy logic that should go away

nothing bad happens when a Hiera 5 datadir path doesn't exist.

hieradata/ is a Hiera ≤4 naming convention.

op-ct · 2020-02-04T21:06:28Z

lib/simp/spec_helpers/helpers.rb

+    end
+
+    if not File.directory?(File.join(fixture_path,'modules',module_name)) then
+      FileUtils.mkdir_p(File.join(fixture_path,'modules',module_name))


This mkdir_p seems unnecessary.

op-ct · 2020-02-04T21:09:31Z

lib/simp/spec_helpers/helpers.rb

+      Puppet::Util::Log.newdestination(:console)
+    end
+
+    default_hiera_config =<<~EOM


We should probably expose a parameter that can override default_hiera_config (or just use fixtures/hiera.yaml if it already exists). Other wise, this is the only possible Hiera config.

Put in a ticket for improvements. I was trying to centralize it so we could in the futre update from one place not optimize it to the nth degree. I took hat we had and centralized it so when we the latest ticket we could put in the changes to call this and then the next ticket would only require updates to this.

op-ct · 2020-02-05T16:08:28Z

Rakefile

+  }
+end
+
+desc "Run spec tests"


There aren't any spec tests for this task to run.

op-ct · 2020-02-05T16:10:03Z

.travis.yml

+    gemspec: simp-spec-helpers.gemspec
+    gem: simp-spec-helpers
+    api_key:
+      secure: "need key"


Thanks for the reminder--I'll update this with a Travis variable in a later PR.

op-ct · 2020-02-05T16:21:32Z

lib/simp/spec_helpers/version.rb

@@ -1,5 +1,5 @@
 module Simp; end

 module Simp::SpecHelpers
-  VERSION = '0.0.1'
+  VERSION = '0.0.2'


This patch adds new API functionality.

Suggested change

VERSION = '0.0.2'

VERSION = '0.1.0'

op-ct · 2020-02-05T16:29:40Z

Gemfile

+end
+
+group :test do
+  gem 'rspec'


rspec is already included by the .gemspec file. Unless the Gemfile's gem statement refines a gemspec's gem (like how PUPPET_VERSION refines the puppet gem) there shouldn't be any redundant gems.

op-ct · 2020-02-05T16:32:00Z

Rakefile

+
+
+desc 'Ensure gemspec-safe permissions on all files'
+task :chmod do


This task should be defined alongside pkg:gem

jeannegreulich · 2020-02-27T16:02:56Z

I let Chris write his own update.

SPECHELPER

1c225e0

- got spec_helpers.rb from pupmod-simp-pupmod - moved normalize function from pupmod tests to spec_helper - create global_spec_helper(dir, modulename) function to call from spec_helper.rb.

jeannegreulich changed the title ~~(SIMP-3430) Create Centralized spec_helper.rb~~ (SIMP-3430) Create Centralized spec_helper.rb (WIP) Jan 24, 2020

jeannegreulich force-pushed the spechelper branch 2 times, most recently from aa608a7 to 685d898 Compare January 27, 2020 21:04

jeannegreulich changed the title ~~(SIMP-3430) Create Centralized spec_helper.rb (WIP)~~ (SIMP-3430) Create Centralized spec_helper.rb Jan 27, 2020

jeannegreulich force-pushed the spechelper branch 3 times, most recently from 8c387b0 to 43691aa Compare January 29, 2020 22:06

jeannegreulich force-pushed the spechelper branch from 43691aa to feb56d5 Compare January 29, 2020 22:14

fix rake files.

1807a25

op-ct suggested changes Feb 5, 2020

View reviewed changes

jeannegreulich closed this Feb 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(SIMP-3430) Create Centralized spec_helper.rb #2

(SIMP-3430) Create Centralized spec_helper.rb #2

jeannegreulich commented Jan 24, 2020

op-ct left a comment •

edited

Loading

op-ct Feb 4, 2020

op-ct Feb 4, 2020

jeannegreulich Feb 7, 2020

op-ct Feb 25, 2020

op-ct Feb 4, 2020

op-ct Feb 4, 2020

op-ct Feb 4, 2020

jeannegreulich Feb 27, 2020

op-ct Feb 5, 2020

op-ct Feb 5, 2020

op-ct Feb 5, 2020

op-ct Feb 5, 2020

op-ct Feb 5, 2020

jeannegreulich commented Feb 27, 2020



		desc 'Ensure gemspec-safe permissions on all files'
		task :chmod do

(SIMP-3430) Create Centralized spec_helper.rb #2

(SIMP-3430) Create Centralized spec_helper.rb #2

Conversation

jeannegreulich commented Jan 24, 2020

op-ct left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeannegreulich commented Feb 27, 2020

op-ct left a comment •

edited

Loading