Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support to override the allowed test tiers #253

Merged
merged 1 commit into from
Jun 29, 2018

Conversation

b4ldr
Copy link
Contributor

@b4ldr b4ldr commented Jun 28, 2018

This PR adds support for a new environment variable TEST_TIERS_ALLOWED to allow users to override the default list of allowed test teirs

expect { described_class.setup_beaker(task) }.to raise_error(RuntimeError, %r{not a valid test tier})
end
it 'Override TEST_TIERS_ALLOWED' do
ENV['TEST_TIERS_ALLOWED'] = 'dev,rnd'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please set up mocking like in the next line, instead of altering the global environment.

Background: using the allow(ENV).to receive(:[]).with('TEST_TIERS').and_return('dev') syntax means that everything is cleaned up in all cases. Modifying global state means that when errors happen, cleanup is skipped, leading to flaky tests.

@@ -64,10 +64,11 @@ def self.setup_beaker(t)
# TEST_TIERS env variable is a comma separated list of tiers to run. e.g. low, medium, high
if ENV['TEST_TIERS']
test_tiers = ENV['TEST_TIERS'].split(',')
raise 'TEST_TIERS env variable must have at least 1 tier specified. low, medium or high (comma separated).' if test_tiers.count == 0
test_tiers_allowed = ENV.fetch('TEST_TIERS_ALLOWED', 'low,medium,high').split(',')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 !

Copy link
Contributor

@DavidS DavidS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, clicked the wrong button there. Please see comments above

@b4ldr
Copy link
Contributor Author

b4ldr commented Jun 28, 2018

@DavidS My rspec foo is not that good and i was unable to get it to work using the following

    it 'Override TEST_TIERS_ALLOWED' do
      allow(ENV).to receive(:[]).with('TEST_TIERS_ALLOWED').and_return('dev,rnd')
      allow(ENV).to receive(:[]).with('TEST_TIERS').and_return('dev')
      expect(described_class.setup_beaker(task).rspec_opts.to_s).to match(%r{--tag tier_dev})
    end

Im not entirely sure why allow(ENV).to receive(:[]).and_return('foobar') works without using the .with modifier. if you are able to give some pointers i would be happy to make the changes

Thanks John

@puppetcla
Copy link

CLA signed by all contributors.

@DavidS
Copy link
Contributor

DavidS commented Jun 28, 2018

@b4ldr I'll have 👀 on that tomorrow.

@DavidS
Copy link
Contributor

DavidS commented Jun 28, 2018

ah, actually, that's easy. because your code is using ENV.fetch(), not ENV[]. If you want to play a bit more with it feel free, otherwise I can fix it up easily tomorrow.

@b4ldr
Copy link
Contributor Author

b4ldr commented Jun 29, 2018

@DavidS thanks that was an easy one :), let me know if you want any other changes

@codecov-io
Copy link

codecov-io commented Jun 29, 2018

Codecov Report

Merging #253 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #253      +/-   ##
=========================================
+ Coverage   39.22%   39.3%   +0.08%     
=========================================
  Files          10      10              
  Lines         719     720       +1     
=========================================
+ Hits          282     283       +1     
  Misses        437     437
Impacted Files Coverage Δ
lib/puppetlabs_spec_helper/tasks/beaker.rb 52.3% <100%> (+0.74%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0326062...0de5995. Read the comment docs.

@DavidS DavidS merged commit 2c887fc into puppetlabs:master Jun 29, 2018
@DavidS
Copy link
Contributor

DavidS commented Jun 29, 2018

Thank you for your time and work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants