Enable and fix rubocop violations

[rski]

As of https://github.com/voxpupuli/puppet-network/commit/ea7cac426ea123629821e49d88cd305f7afe3f23 some checks are disabled. These have to be enabled and fixed.

[alexjfisher]

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

In isolated unit specs, you want each example to specify one (and only one) behavior. Multiple expectations in the same example are a signal that you may be specifying multiple behaviors.

I think it could be argued that we're only testing one behaviour ie 'values are in order'

From http://programmers.stackexchange.com/questions/7823/is-it-ok-to-have-multiple-asserts-in-a-single-unit-test

Tests should fail for one reason only, but that doesn't always mean that there should be only one Assert statement. IMHO it is more important to hold to the "Arrange, Act, Assert" pattern.

The key is that you have only one action, and then you inspect the results of that action using asserts. But it is "Arrange, Act, Assert, End of test". If you are tempted to continue testing by performing another action and more asserts afterwards, make that a separate test instead.

I am happy to see multiple assert statements that form parts of testing the same action...

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! :)

[rski]

Ah, good point. If there is no meaningful semantics there is no point.