getsaf / shallow-render

Angular testing made easy with shallow rendering and easy mocking. https://getsaf.github.io/shallow-render
MIT License
273 stars 25 forks source link

[Regression][v9] adding stubs to child components causes child component "@Output" removal #159

Closed hakimio closed 4 years ago

hakimio commented 4 years ago

After upgrading to version 9 of shallow-render and Angular, using shallow.mock() to add child component instance properties causes @Output fooChange not initialized in 'MockOfFooComponent' error. Without calling shallow.mock() component is initialized correctly with child component outputs.

hakimio commented 4 years ago

The issue seems to be extendMockWithStubs. Why don't you just use Object.defineProperties()?

hakimio commented 4 years ago

Here is a possible fix:

const extendMockWithStubs = (mock: Type<any>, stubs: object, className: string): void => {
  const newProperties = Object.keys(stubs)
        .reduce((result, key) => (result[key] = {
            value: stubs[key],
            writable: true
        }, result), {});
    Object.defineProperties(mock.prototype, newProperties);
    Object.defineProperty(mock, 'name', { value: className });
};

Should I create a proper PR or can you do it?

getsaf commented 4 years ago

Thanks for posting, go ahead and submit the pr, I'll get it verified soon and get a quick release out.

Glad to have another contributor.

getsaf commented 4 years ago

Oh, and if you are up to it, try adding a test for the issue that the new code change fixes.

hakimio commented 4 years ago

Ok, I'll make a PR with a fix and a unit test soon. Thank you very much for making this awesome library :)

getsaf commented 4 years ago

Been looking at this issue this morning.. The reason extendMockWithStubs method is extending rather than simply defining properties is because Component mocks from ng-mocks are memoized (i.e. every time you ask for an ng-mock of a component, you get the exact same instance back.

eg:

MockComponent(FooComponent) === MockComponent(FooComponent) // is true

I need to make sure that I don't define properties on the memoized value or the new properties will bleed into other tests that mock the same component.

To get around that, I extend the ng-mock class and add stubs to the extended class. I had some issues extending the class and couldn't figure out what the issue was during the v9 upgrade.

I was trying to do:

class ExtendedMock extends MockComponent(FooComponent) {
   ...
}

Which was not working properly so I copied over the prototype instead.. Issue now is that the original mock's constructor is not firing...

I don't have the solution just yet, but I'm looking at it too. Just wanted to give you some of my thoughts early in case you found something else useful.

getsaf commented 4 years ago

I found a fix!! Gonna push up a PR in a few.

hakimio commented 4 years ago

Awesome :)

getsaf commented 4 years ago

Released v9.0.4.

Give it a go and if it all works out, close this issue. Thanks again for reporting the issue!

hakimio commented 4 years ago

Yes, it's working well now. Thanks for fixing the issue.