Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem deep comparing two immutable Lists #23

Closed
garethmdavies opened this issue Oct 8, 2015 · 12 comments
Closed

Problem deep comparing two immutable Lists #23

garethmdavies opened this issue Oct 8, 2015 · 12 comments
Labels

Comments

@garethmdavies
Copy link

Given a couple of immutable lists with identical items in some instances I've had problems when deep comparing them in my tests using the chai-immutable plugin.

  expect(Immutable.is(listOne, listTwo)).to.equal(true); // works ok
  expect(listOne).is.eql(listTwo); // fails
@astorije
Copy link
Owner

Hi @taffowl,

What version of chai-immutable are you using?
Also, what are listOne and listTwo?

Using primitives, the following works:

it('test issue #23', function () {
  var listOne = List.of(1, 2, 3);
  var listTwo = List.of(1, 2, 3);
  expect(Immutable.is(listOne, listTwo)).to.equal(true);
  expect(listOne).to.equal(listTwo);
  expect(listOne).to.eql(listTwo);
});

I am guessing this works for primitives but fails for nested structures, as described in #24.

@astorije
Copy link
Owner

Odd, this passes fine too:

      it('test issue #23', function () {
        var listOne = List.of(1, 2, (List.of(5, 4)));
        var listTwo = List.of(1, 2, (List.of(3, 4)));
        expect(Immutable.is(listOne, listTwo)).to.equal(true);
        expect(listOne).to.equal(listTwo);
        expect(listOne).to.eql(listTwo);
      });

@astorije
Copy link
Owner

@taffowl, any chance you could add precisions to your case? In particular, do #24 and #25 address your issue, somehow?

If I do not get answers to this comment, I'm afraid I'll have to close this issue. Feel free to add precisions here in the future if you think I am being mistaken.

@garethmdavies
Copy link
Author

@astorije Hi, sorry for the delayed response I was at a bit of a loss to the underlying problem. I think issues #24 and #25 pretty much describes what was happening in that in my case I was adding mutable objects and not primitives into the immutable list. I've switched my test case to use "to.equal".

The chai-immutable version I'm using is 1.3.0. Below is an example test that I've just pulled together that demonstrates the issue.

var chai = require('chai');
var expect = chai.expect;
chai.use(require('chai-immutable'));
var Immutable = require('Immutable');

function parseFlatErrors(errors, mappings) {
  return Immutable.List(errors
      .filter(error => !!mappings[error.code])
      .map(error => mappings[error.code])
  );
};

describe('response error mapping', () => {

  /* Server response with list of errors

   var response = {
    statusCode: 500, body: {
      data: {
        code: "validation.failure",
        type: "Failed resource validation",
        errors: [
          {code: 'error1'},
          {code: 'error2'},
          {code: 'error3'},
          {code: 'foo.error1'},
          {code: 'foo.error2'},
          {code: 'bar.error1'}
        ]
      },
      status: "error"
    }
  };
  */

  it('should parse flat errors', () => {

    var flatErrorMapper = {
      'error1': 'message.error1',
      'error2': 'message.error2',
      'error3': 'message.error3',
      'foo.error1': 'message.foo.error1',
      'foo.error2': 'message.foo.error2',
      'bar.error1': 'message.bar.error1'
    };

    var errors = [
      {code: 'error1'},
      {code: 'error2'},
      {code: 'error3'},
      {code: 'foo.error1'},
      {code: 'foo.error2'},
      {code: 'bar.error1'}
    ];

    var result = parseFlatErrors(errors, flatErrorMapper);

    var expected = Immutable.List.of(
      flatErrorMapper.error1,
      flatErrorMapper.error2,
      flatErrorMapper.error3,
      flatErrorMapper['foo.error1'],
      flatErrorMapper['foo.error2'],
      flatErrorMapper['bar.error1']
    );

    console.log("Result: ", result);
    console.log("Expected: ", expected);

    expect(Immutable.is(result, expected)).to.equal(true); // Works
    expect(result).is.eql(expected); // fails
    expect(result).to.equal(expected); // Works
  });

});

@astorije
Copy link
Owner

Hum, I have run your test case on master, the branch behind #25, and on v1.3.0, and all tests were passing, every time... Is there something specific I need to do to reproduce this bug? What version of Immutable.js are you using?

@garethmdavies
Copy link
Author

That is strange, the output I get when running the tests is below. I'm using Immutable version: 3.7.4 and chai version: 2.3.0. I can't think of anything else but I'll take another look tomorrow.

  response error mapping
Result:  List [ "message.error1", "message.error2", "message.error3", "message.foo.error1", "message.foo.error2", "message.bar.error1" ]
Expected:  List [ "message.error1", "message.error2", "message.error3", "message.foo.error1", "message.foo.error2", "message.bar.error1" ]
    1) should parse flat errors


  0 passing (196ms)
  1 failing

  1) response error mapping should parse flat errors:

      AssertionError: expected { Object (size, _origin, ...) } to equal { Object (size, _origin, ...) }
      + expected - actual

@astorije
Copy link
Owner

That's bizarre indeed, I was using Chai 3.x but tested with the following environment and... all tests are passing for me!

chai-immutable@1.3.0
├── chai@2.3.0
├── coveralls@2.11.4
├── immutable@3.7.4
├── istanbul@0.3.17
├── jscs@2.1.1
├── mocha@2.2.5
├── mocha-phantomjs@3.6.0
└── phantomjs@1.9.7-15

(I used npm list --depth 0 to get this list)

@garethmdavies
Copy link
Author

My current environment is part of a much bigger piece of work so I have 72 dependencies when I run npm list --depth 0 (so I guess scope for issues). I'll build another environment and use the core dependencies and re-run the tests to see if I still have the problem.

@astorije
Copy link
Owner

I have tried your test suite in a different folder, with freshly installed packages, and this time I have it failing.

I note however that among your 3 tests:

    expect(Immutable.is(result, expected)).to.equal(true); // Works
    expect(result).is.eql(expected); // fails
    expect(result).to.equal(expected); // Works

The one that is failing is the last one, not the other two!

And that's where things look shady. If I had your test suite in my test file, things pass just fine. The difference is that in my case, this evaluates to true, in your case it evaluates to false.

@astorije
Copy link
Owner

Oh my... I have found the issue.

The fourth line of your snippet is:

var Immutable = require('Immutable');

where it should be:

var Immutable = require('immutable');

Notice the lowercase i?

I have no idea why this is acting the way it is honestly. Am I doing something wrong in chai-immutable? Is require buggy? Is mocha or chai buggy?
Frankly I have no idea. I tend to think this is chai-immutable's fault, but I'm not sure why, but it seems to work fine when using the right package names (immutable vs. Immutable) so I'm not sure it's worth investigating. Unless I am violating something somewhere here in which case I'd be happy to fix with some help.

@astorije
Copy link
Owner

@taffowl, can you confirm this was user-side error (the wrong case) and this issue can be closed?

@garethmdavies
Copy link
Author

Sorry, yes changing the immutable to a lowercase within my particular test case resolved the issue. I'm still not sure what's happening behind the scenes or why the ide didn't pick up the typo. Oh well at least if somebody else hits this it's easy to resolve. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants