Skip to content

Commit

Permalink
Add some more all blank changes tests
Browse files Browse the repository at this point in the history
Originally there was a single test for all blank changes. This commit adds a
number more. Now it tests:

  - Both [nil, <blank thing>] and [<blank thing>, nil] changes.
  - <blank thing> was originally just an empty Array. Now it can be:
    - []
    - false
    - ''
    - A non-empty String consisting of all whitespace characters
    - {}
  - It tests with the track_blank_changes option the default, explicitly
    false, and explicitly true.

The tests with track_blank_changes the default or explicitly false succeed,
accidentally for now, as there is no such option and the code acts as if
track_blank_changes is false, so tests that rely on that succeed. The tests
that set track_blank_changes to true fail, as there's no such option and
setting it has no effect on the operation of the code.

Here's a diff of the test results with just the important parts, not all 422
lines:

  $ diff -u testresults/1-track_blank_changes-default-tests.txt testresults/2-addition-all-blank-changes.txt
  --- testresults/1-track_blank_changes-default-tests.txt	2024-08-05 11:21:01.000000000 -0700
  +++ testresults/2-addition-all-blank-changes.txt	2024-08-05 11:34:14.000000000 -0700
  -432 examples, 6 failures, 2 pending
  +455 examples, 14 failures, 2 pending

The 6 failures are those introduced by commit 52cc180. The 8 additional
failures are introduced in this commit.

And, just to verify, I commented out the unless clause on
lib/mongoid/history/attributes/update.rb:29, which makes track_blank_changes
effectively always true, and the tests that don't set it or set it to false
fail (as it's always implicitly true with the commented out unless), and the
tests that set it to true succeed (the option isn't being set, there is no
option, but the code acts as if it's true), all as I'd expect things to
work.
  • Loading branch information
BrianLMatthews committed Aug 5, 2024
1 parent 52cc180 commit 8081c34
Showing 1 changed file with 61 additions and 29 deletions.
90 changes: 61 additions & 29 deletions spec/unit/attributes/update_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -305,37 +305,69 @@ class EmbOne
end
end

context 'when original and modified values blank' do
before :each do
class DummyParent
include Mongoid::Document
include Mongoid::History::Trackable
store_in collection: :dummy_parents
has_and_belongs_to_many :other_dummy_parents
track_history on: :fields
end

class OtherDummyParent
include Mongoid::Document
has_and_belongs_to_many :dummy_parents
end
end

before :each do
allow(base).to receive(:changes) { changes }
DummyParent.track_history on: :other_dummy_parent_ids
end
[false, true].each do |original_nil|
context "when original value #{original_nil ? 'nil' : 'blank'} and modified value #{original_nil ? 'blank' : 'nil'}" do
[nil, false, true].each do |track_blank_changes|
context "when track_blank_changes #{track_blank_changes.nil? ? 'default' : track_blank_changes}" do
before :each do
class DummyParent
include Mongoid::Document
include Mongoid::History::Trackable
store_in collection: :dummy_parents
has_and_belongs_to_many :other_dummy_parents
field :boolean, type: Boolean
field :string, type: String
field :hash, type: Hash
end

class OtherDummyParent
include Mongoid::Document
has_and_belongs_to_many :dummy_parents
end

if track_blank_changes.nil?
DummyParent.track_history on: :fields
else
DummyParent.track_history \
on: :fields,
track_blank_changes: track_blank_changes
end

allow(base).to receive(:changes) { changes }
end

let(:base) { described_class.new(DummyParent.new) }
let(:changes) do
{ 'other_dummy_parent_ids' => [nil, []] }
end
subject { base.attributes }
it { expect(subject.keys).to_not include 'other_dummy_parent_ids' }
after :each do
Object.send(:remove_const, :DummyParent)
Object.send(:remove_const, :OtherDummyParent)
end

after :each do
Object.send(:remove_const, :DummyParent)
Object.send(:remove_const, :OtherDummyParent)
let(:base) { described_class.new(DummyParent.new) }
subject { base.attributes.keys }

# These can't be memoizing methods (i.e. lets) because of limits
# on where those can be used.

cmp = track_blank_changes ? 'should' : 'should_not'
cmp_name = cmp.humanize capitalize: false

[
{ n: 'many-to-many', f: 'other_dummy_parent_ids', v: [] },
{ n: 'boolean', f: 'boolean', v: false },
{ n: 'empty string', f: 'string', v: '' },
{ n: 'all whitespace string', f: 'string', v: " \t\n\r\f\v" }
# The second character in that string is an actual tab (0x9).
].each do |d|
context "#{d[:n]} field" do
let(:changes) do
{ d[:f] => original_nil ? [nil, d[:v]] : [d[:v], nil] }
end
it "changes #{cmp_name} include #{d[:f]}" do
send(cmp, include(d[:f]))
end
end
end
end
end
end
end
end
Expand Down

0 comments on commit 8081c34

Please sign in to comment.