voxpupuli / puppet-network

Types and providers to manage network interfaces
https://forge.puppet.com/puppet/network
Apache License 2.0
66 stars 108 forks source link

Remove with_env, and trust in PATH being correct #177

Closed igalic closed 8 years ago

igalic commented 8 years ago

Modern versions of Facter (v3) don't have .with_env(). In this first step of modernising our facts, we remove the .with_env() block and simply trust that the PATH is correct and that we'll be able to find ip(8).

In a next step, we should replace Facter::Util::Resolution with Facter::Core::Execution. note that this will require reworking the spec tests which rely on different sets of external facts that do use Facter::Util::Resolution.

igalic commented 8 years ago

:arrow_up: this happened because cat walked over my keyboard as i was submitting the pr :arrow_up:

igalic commented 8 years ago

after this commit, we're down to:

Running RuboCop...
Inspecting 25 files
..............CCCC.......

Offenses:

spec/unit/provider/network_route/redhat_spec.rb:137:9: C: RSpec/MultipleExpectations: Too many expectations. 
        it 'writes 5 fields' do
        ^^^^^^^^^^^^^^^^^^^^
spec/unit/provider/network_route/routes_spec.rb:117:7: C: RSpec/MultipleExpectations: Too many expectations. 
      it 'writes all 5 fields' do
      ^^^^^^^^^^^^^^^^^^^^^^^^
spec/unit/provider/network_route/routes_spec.rb:169:7: C: RSpec/MultipleExpectations: Too many expectations. 
      it 'writes only fields' do
      ^^^^^^^^^^^^^^^^^^^^^^^
spec/unit/provider/network_config/redhat_spec.rb:30:5: C: RSpec/LeadingSubject: Declare subject above any other let declarations 
    subject { described_class.target_files(network_scripts_path).map { |file| File.basename(file) } }
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/unit/provider/network_config/interfaces_spec.rb:113:5: C: RSpec/MultipleExpectations: Too many expectations. 
    it 'parses out vlan iface lines' do
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spec/unit/provider/network_config/interfaces_spec.rb:375:9: C: RSpec/MultipleExpectations: Too many expectations. 
        it 'writes the values in order' do
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

25 files inspected, 6 offenses detected
RuboCop failed!
igalic commented 8 years ago

not quite sure yet how to deal with those


spec/unit/provider/network_route/redhat_spec.rb:137:9: C: RSpec/MultipleExpectations: Too many expectations. 
        it 'writes 5 fields' do
        ^^^^^^^^^^^^^^^^^^^^

it confuses, that _this_ breaks, but the group underneath doesn't.

alexjfisher commented 8 years ago

it 'writes 5 fields' No it doesn't... It writes 7, but the test got fudged a while back... https://github.com/voxpupuli/puppet-network/commit/1e849010ba7241c407bead741229706ecee770a4#diff-345d8abd3a600d025a0e021a64c4941fR100

rski commented 8 years ago

@alexjfisher that's weird because the tests used to pass

alexjfisher commented 8 years ago

Sure, they pass, but the split(' ', 5).length).to eq(5) is hardly a surprise!

alexjfisher commented 8 years ago

options were added here https://github.com/voxpupuli/puppet-network/commit/1e849010ba7241c407bead741229706ecee770a4#diff-345d8abd3a600d025a0e021a64c4941fR90 and this changed the output from "172.17.67.0/30 via 172.18.6.2 dev vlan200" (5 fields) to "172.17.67.0/30 via 172.18.6.2 dev vlan200 table 200" (7 fields)

The it "should write 5 fields" test was 'fixed' with

-          content.scan(/^172.17.67.0\/30 .*$/).first.split(' ').length.should == 5
+          content.scan(/^172.17.67.0\/30 .*$/).first.split(' ', 5).length.should == 5

instead of changing it to a it "should write 7 fields" test.

rski commented 8 years ago

tsk. Ok, give me a sec and I'll un-funge it. Then @igalic can rebase on that I guess

alexjfisher commented 8 years ago

@rski I was about to submit a PR myself...

alexjfisher commented 8 years ago

https://github.com/voxpupuli/puppet-network/pull/178

rski commented 8 years ago

whoops, sorry

alexjfisher commented 8 years ago

@rski No worries!

@igalic https://github.com/voxpupuli/puppet-network/pull/178 also fixes the rubocop violation.