rochejul / sequelize-mocking

Sequelize extension to deal with data-mocking for testing
MIT License
63 stars 26 forks source link

Issue mocking with multiple models #14

Closed RobRendell closed 6 years ago

RobRendell commented 6 years ago

Hi! If I have multiple models loaded in my sequelize object, sequelize-mocking appears to only copy one of them (the last one it processes).

sequelize version: 4.4.2 sequelize-mocking version: 1.0.0

Minimal test code which fails for me:

const chai = require('chai');
const sequelizeMockingMocha = require('sequelize-mocking').sequelizeMockingMocha;
const path = require('path');
const Sequelize = require('sequelize');

describe('User class', function () {

    const sequelize = new Sequelize('mysql://user:xyzzy@localhost:3306/');
    const User = sequelize.define('User', {});
    const OtherObject = sequelize.define('OtherObject', {});

    sequelizeMockingMocha(
        sequelize,
        path.resolve(path.join(__dirname, './test.json')),
        {'logging': false}
    );

    it('should exist', function () {
        chai.expect(User).to.exist;
    });
});

test.json file: [ { "model": "User", "data": { } } ]

Running this test produces the following error for me:

1) User class "before each" hook for "should exist": Error: Model not found: User at Loader.loadFixture (node_modules\sequelize-fixtures\lib\loader.js:37:15) at . (node_modules\sequelize-fixtures\lib\loader.js:12:21)`

Commenting out the "const OtherObject" line causes the test to pass.

Drilling into the sequelize-mocking code, it appears that the method sequelize-mocking.js#copyCurrentModels doesn't actually manage to copy all the models from the originalSequelize to the mockedSequelize. It iterates over them all, but only the last one processed ends up in the mockedSequelize's ModelManager.

Thanks!

rochejul commented 6 years ago

Ok I will take a look on that. Probably a regression due to sequelize 4 migration

Regards

RobRendell commented 6 years ago

Thanks!

Thanks! Do you have any thoughts about when 1.0.1 might become available?

For now, we can work around the issue by running the test files in isolation (mocha test/.../singleModelTest.js). However, eventually we'll want to have tests that involve multiple models at once.

RobRendell commented 6 years ago

Ok, I've found a fix for this issue that works for me. I've overridden the definition of SequelizeMocking.copyModel with the following version:

SequelizeMocking.copyModel = function copyModel(sequelizeInstance, model) {
    let newModel = class extends Sequelize.Model {};
    newModel.init(
        _.merge({ }, model.attributes),
        _.merge({ }, model.options, { 'sequelize': sequelizeInstance, 'modelName': model.name })
    );
    return newModel;
};

This was based on the code in Sequelize.define().

abemedia commented 6 years ago

Hi, I have the same issue but when using the patch by @RobRendell in ES6 I get the error TypeError: Class constructor Model cannot be invoked without 'new'. Any ideas how to do this in ES6?

rochejul commented 6 years ago

Hi

Thanks @RobRendell and @abeMedia

I try to look on it on the next week (I was on vacations)

Many thanks for your feedback

Regards

rochejul commented 6 years ago

Ok, it seems with this code:

static copyModel(sequelizeInstance, model) {
        class TempModel extends Sequelize.Model {
        }

        let newModel = TempModel.init(
            _.merge({ }, model.attributes),
            _.merge({ }, model.options, { 'sequelize': sequelizeInstance, 'modelName': model.name })
        );
        return newModel;
    }

It seems working fine now (very weird that a "static" method has got a context that could be changed)

I try to patch it as soon as possible

Regards