Skip to content

Commit

Permalink
Unit test that demonstrates bug in state update logic
Browse files Browse the repository at this point in the history
This commit demonstrates a bug in state handling. Every modification involves a read, modify, write. This unit test mocks out just enough to show that a state file with ssl -> true is read correctly, changed, then produces an invalid json file with two entries in the hash with the same key. This is the key issue as reported in facebook#115.

```
[malmond@devvm1846.vll1 ~/local/taste-tester]
/opt/chefdk/embedded/bin/rspec .
F

Failures:

  1) TasteTester::State should serialize changes correctly
     Failure/Error: expect(@buffer.string).to eq('{"ssl":false}')

       expected: "{\"ssl\":false}"
            got: "{\"ssl\":true,\"ssl\":false}"

       (compared using ==)
     # ./spec/taste-tester_spec.rb:20:in `block (2 levels) in <top
     # (required)>'

Finished in 0.01544 seconds (files took 0.27334 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/taste-tester_spec.rb:6 # TasteTester::State should serialize changes correctly
```

Signed-off-by: Matthew Almond <malmond@fb.com>
  • Loading branch information
malmond77 committed Aug 19, 2019
1 parent 6cb5f8f commit 954d063
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 6 deletions.
9 changes: 3 additions & 6 deletions lib/taste_tester/state.rb
Expand Up @@ -145,12 +145,9 @@ def merge(vals)
state = {}
end
state.merge!(vals)
ff = File.open(
TasteTester::Config.ref_file,
'w',
)
ff.write(state.to_json)
ff.close
File.open(TasteTester::Config.ref_file, 'w') do |ff|
ff.write(state.to_json)
end
rescue StandardError => e
logger.error('Unable to write the reffile')
logger.debug(e)
Expand Down
23 changes: 23 additions & 0 deletions spec/taste-tester_spec.rb
@@ -0,0 +1,23 @@
require "spec_helper"
require 'taste_tester/state'

describe TasteTester::State do

it "should serialize changes correctly" do
# initializing a state object creates a directory
# so let's pretend it already exists.
allow(File).to receive(:directory?).and_return(true)

# original state file says ssl -> true
allow(File).to receive(:read).and_return('{"ssl": true}')
@s = TasteTester::State.new

expect(@s.ssl).to eq(true)

@buffer = StringIO.new()
allow(File).to receive(:open).and_yield(@buffer)
@s.ssl = false
expect(@buffer.string).to eq('{"ssl":false}')
end

end

0 comments on commit 954d063

Please sign in to comment.