-
-
Notifications
You must be signed in to change notification settings - Fork 108
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
Enable and fix rubocop violations #179
Comments
have to? I'm not so sure (but would be happy to be convinced otherwise). I did consider fixing the violations, but couldn't think of a nice way that actually improved the tests. eg in it 'writes the values in order' do # rubocop:disable RSpec/MultipleExpectations
expect(content.scan(%r{^\s*post-down .*$})[0]).to eq(' post-down /bin/touch /tmp/eth1-down1')
expect(content.scan(%r{^\s*post-down .*$})[1]).to eq(' post-down /bin/touch /tmp/eth1-down2')
end From http://betterspecs.org/#single
I think it could be argued that we're only testing one behaviour ie 'values are in order'
In summary, I don't think the test would have been improved if I'd changed it to describe 'ordering of values' do
it 'the first value comes first' do
expect(content.scan(%r{^\s*post-down .*$})[0]).to eq(' post-down /bin/touch /tmp/eth1-down1')
end
it 'the second value comes second'
expect(content.scan(%r{^\s*post-down .*$})[1]).to eq(' post-down /bin/touch /tmp/eth1-down2')
end
end But maybe I just couldn't think of the right words to use. Replace 'the first value comes first' with something more meaningful and maybe I'll change my mind! :) |
Ah, good point. If there is no meaningful semantics there is no point. |
As of ea7cac4 some checks are disabled. These have to be enabled and fixed.
The text was updated successfully, but these errors were encountered: