-
Notifications
You must be signed in to change notification settings - Fork 71
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
bump puppetlabs_spec_helper + add needed dependency #319
Conversation
Have you reviewed how the output looks? See voxpupuli/puppet-collectd#633 where I ran some tests against collectd. It's fine when everything is green, but when there are errors, it's ... not pretty. |
In my opinion this is okay. It is harder to read, yes, but the speed gain is huge. We can still run the tests without parallel if it gets to ugly for debugging. |
puppetlabs_spec_helper got released so in theorie we could merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is requiring parallel_tests
basically a switch for turning that feature on, or why isn't it just in puppetlabs_spec_helper
?
@@ -17,7 +17,8 @@ Gemfile: | |||
required: | |||
':test': | |||
- gem: puppetlabs_spec_helper | |||
version: '~> 1.2.2' | |||
version: '~> 2.0.1' | |||
- gem: parallel_tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't understand why this gem isn't just a transitive dependency that we don't have to care for, because puppetlabs_spec_helper
depends on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, an earlier draft of the feature made the test target always run in parallel, which was my concern. Glad it's optional. |
I tested this locally with puppet-zabbix. Works great so far, spec tests run now in parallel. However we should hold of with merging, puppetlabs_spec_helper seems to be broken in certain situations. puppetlabs/puppetlabs_spec_helper#177