soumak77 / firebase-mock

Firebase mock library for writing unit tests
https://soumak77.github.io/firebase-mock
350 stars 96 forks source link

Updating nested value via a transaction leads to inconsistent data #60

Closed a-xin closed 6 years ago

a-xin commented 6 years ago

Adding the following test to test/unit/firebase.js (MockFirebase #transaction):

it('should allow updating nested values in transactions', function () {
  ref.set({
    item: {
      meta: {
        time: 1234,
      },
    },
  });
  ref.flush();

  ref.child('item').transaction(function(value) {
    if (value) {
      value.meta.time = 9999;
    }

    return value;  
  });
  ref.flush();

  expect(ref.getData().item).to.equal(ref.child('item').getData());
});

Fails with:

AssertionError: expected { meta: { time: 1234 } } to equal { meta: { time: 9999 } }
+ expected - actual

  {
    "meta": {
-    "time": 1234
+    "time": 9999
    }
  }

I would have expected both paths to return the updated data and therefore the test to pass.

a-xin commented 6 years ago

Hey @soumak77 I'm happy to take a look at it. Do you happen to have any suspicion what could be causing this issue?

soumak77 commented 6 years ago

@a-xin I think I just found the issue. I'm refactoring the code to use lodash 4.x and stumbled across this code: https://github.com/soumak77/firebase-mock/blob/1c348c14b812a20a15ae388352413f12bac63790/src/firebase.js#L340-L358

Looks like I didn't replace this with self in all places. This change will be made with my other changes to support lodash 4.x.

soumak77 commented 6 years ago

@a-xin please see if this has been resolved with v2.1.3

soumak77 commented 6 years ago

@a-xin I found and fixed the root cause. Nested values were not being cloned properly.

a-xin commented 6 years ago

@soumak77 Thanks, great you caught the root cause!