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

ComponentFactoryResolver and its mocks #736

Closed
satanTime opened this issue Jun 21, 2021 · 28 comments · Fixed by #751
Closed

ComponentFactoryResolver and its mocks #736

satanTime opened this issue Jun 21, 2021 · 28 comments · Fixed by #751

Comments

@satanTime
Copy link
Member

Alright! Is it the same thing for ComponentFactoryResolver?
Actually, about the dom sanitizer, I don't want to mock it, but to spy it. If you have better solutions, I would take it!

Originally posted by @GerkinDev in #735 (comment)

@GerkinDev
Copy link

About the component factory resolver mocking, here is my use case:

I have a reusable component (my DynamicComponentHostComponent), who takes as parameter a component factory resolver and a component ctor. This dynamic component host is included in another component, whose job is to lazily and dynamically load a module, eventually compile it, and instanciate it. Then, it passes the component factory resolver to the dynamic host.

To be isolated, I then check in the dynamic host if it calls correctly the input resolver. That's why I need to mock it.


As a side note, this is worth mentionning the concerned component was written before I migrated to @angular/cdk which provides quite the behavior I'm implementing through component portals, so I might remove this component in a future refactor. If the issue is not fixable or brings too many side effects, well, I'll do with it.

@satanTime
Copy link
Member Author

Hi @GerkinDev,

might you give me an example where and how you mock the resolver?

@GerkinDev
Copy link

beforeEach( () => MockBuilder( DynamicComponentHostComponent )
	.provide( {
		provide: ComponentFactoryResolver,
		useFactory: () => ({ resolveComponentFactory: jest.fn(), /* and eventually other used methods */ }) ) );

My explanation above was to explain you that even if I totally get that mocking ComponentFactoryResolver is rather uncommon, it can be sometimes useful, so it should be doable IMO

@satanTime
Copy link
Member Author

Thanks for the example! Totally agree with you.

@satanTime
Copy link
Member Author

Hi @GerkinDev,

might you improve my test case? I couldn't reproduce the issue: https://stackblitz.com/edit/github-qdctmb?file=src%2Ftest.spec.ts

@GerkinDev
Copy link

I'll do that ASAP, I'm off for a few days. But it seems that the test case you wrote is rather similar to my own... So that's awkward.

I'll double check version ranges too.

@satanTime
Copy link
Member Author

satanTime commented Jun 28, 2021

Hi @GerkinDev,

I was thinking about a possible solution like that:

beforeEach(() => MockInstance(ComponentFactoryResolver, 'resolveComponentFactory', jasmine.createSpy()));
beforeEach(() => MockBuilder(DynamicComponentHostComponent).mock(ComponentFactoryResolver));

Do you use pure Angular, or a framework like Ionic?

@GerkinDev
Copy link

Pure angular.

Well, I've not yet tested the solutions suggested above, but I've stumbled upon very awkward behavior I really can't explain.

	beforeEach( () => MockBuilder( DynamicComponentHostComponent )
		.provide( { provide: ComponentFactoryResolver, useFactory: () => {
            const mock = { resolveComponentFactory: jest.fn(), hello: 'world' };
            console.log(mock);
            return mock;
        } } ) );

Here, mock's resolveComponentFactory is as expected a mock function.

But in my component, I got this if I console.log it in the CTOR:

{ hello: 'world', resolveComponentFactory: [Function (anonymous)] }

So I don't know what happened here but it looks like resolveComponentFactory is unmocked. I suspect this behavior to be jest-specific. Have you a stackblitz template configured for jest ?

@satanTime
Copy link
Member Author

I think we might try another solution, I was fighting for the last 2 weeks. Then it covers also platform and root providers. However, I wasn't able to find what ComponentFactoryResolver belongs to.

beforeEach(() => ngMocks.globalMock(ComponentFactoryResolver));
beforeEach(() => ngMocks.defaultMock(ComponentFactoryResolver, () => {
  const mock = { resolveComponentFactory: jest.fn(), hello: 'world' };
  console.log(mock);
  return mock;
}));
afterEach(() => ngMocks.defaultMock(ComponentFactoryResolver));
afterEach(() => ngMocks.globalWipe(ComponentFactoryResolver));

@GerkinDev
Copy link

beforeEach(() => MockInstance(ComponentFactoryResolver, 'resolveComponentFactory', jasmine.createSpy())); // Not working
beforeEach(() => MockBuilder(DynamicComponentHostComponent).mock(ComponentFactoryResolver)); // Neither
beforeEach(() => MockBuilder(DynamicComponentHostComponent)..mock( ComponentFactoryResolver, { resolveComponentFactory: jest.fn(), hello: 'world' } )); // Still nope

@satanTime
Copy link
Member Author

Maybe let's have a call this week to check it live in your project if it is fine, would it work?

@satanTime
Copy link
Member Author

About a template for jest - nope, unfortunately.

But the fact, that you got mock property and the original resolveComponentFactory points that I did a mistake somewhere in code code and the fix for popup overrides the mock resolveComponentFactory with own implementation, however it should respect it.

I'll try to check it deeper today.

@satanTime
Copy link
Member Author

For now, I would blame how ng-mocks handles entryComponents, might you check your test with commented entryComponents declarations?

@satanTime
Copy link
Member Author

satanTime commented Jun 28, 2021

Could you try whether this one works? ng-mocks.zip

@GerkinDev
Copy link

Maybe let's have a call this week to check it live in your project if it is fine, would it work?

Yeah why not, but I admit I'm much more comfortable by messages than orally, I'm pretty bad in English haha

The thing is, https://stackblitz.com/edit/github-qdctmb?file=src%2Ftest.spec.ts have the exact same behavior as I described above, eg. unmocking resolveComponentFactory during injection in my component if I copy/paste it in my test file in my env. And same again when I do not mock the module altogether (which I never do since I prefer explicit dependencies listing). So I highly suspect this is a jest-only behavior.

Footnote: it happened at the same time I updated jest@>=24.0.0

@GerkinDev
Copy link

Could you try whether this one works? ng-mocks.zip

Unfortunatelly, same...

@GerkinDev
Copy link

GerkinDev commented Jun 28, 2021

beforeEach(() => ngMocks.globalMock(ComponentFactoryResolver));
beforeEach(() => ngMocks.defaultMock(ComponentFactoryResolver, () => {
  const mock = { resolveComponentFactory: jest.fn(), hello: 'world' };
  console.log(mock);
  return mock;
}));
afterEach(() => ngMocks.defaultMock(ComponentFactoryResolver));
afterEach(() => ngMocks.globalWipe(ComponentFactoryResolver));

Yet again, this does not work either. Just, WTF is going on ??? 😲 I must have done something stupid somewhere in my codebase. Yet, I do not see any prototype modifications nor assignations...

I can invite you in the repo so you can check if you want.

@satanTime
Copy link
Member Author

I can invite you in the repo so you can check if you want.

It would be great, I think this is the best option :)

@satanTime
Copy link
Member Author

I think this is the problem: https://take.ms/1toX6

with the repo, it should be straight forward to eliminate the problem.

@satanTime
Copy link
Member Author

Got the invitation. Thanks! I'll take a look tonight.

@GerkinDev
Copy link

GerkinDev commented Jun 28, 2021

So, I guess you'll understand I'll remove your access once the issue is resolved. The file causing the problem is this one: https://github.com/Scitizen/front-office/blob/wip/ng-mocks-wtf/src/app/content-modules/annotation/public/components/views/dynamic-component-host/dynamic-component-host.component.spec.ts
You won't be able to run it, since it relies on other protected repositories you don't have access to.

I think this is the problem: https://take.ms/1toX6

with the repo, it should be straight forward to eliminate the problem.

Yeah indeed, that's pretty close to be that. But it's strange that it would cause problems with jest but not jasmine. I'm not aware of the underlying method of mocking of jasmine and jest, their differences, and why exactly one would work but not the other.

@satanTime
Copy link
Member Author

Yep, sure, makes sense!

And the last attempt before a deep dive :) Could you check it? ng-mocks.zip

@GerkinDev
Copy link

GerkinDev commented Jun 28, 2021

Hurray

Ok now you're the best ;)

Can you please mention the issue in the related commit, so that I can look what you've done ?

@satanTime
Copy link
Member Author

Sure. Tonight I'll prepare a PR. This is the change: https://take.ms/zYNDD.

@GerkinDev
Copy link

Yeaaaah obviously haha
But I still don't understand why the issue does not occurs on jasmine 🤔

Btw, what IDE/extension are you using ? Parameters names are neat

@satanTime
Copy link
Member Author

Yep, this is the magic I'll try to cover with UT tonight, and then we know all the dices.

I use IntelliJ IDEA. Also, their Webstorm should have the same feature.

@satanTime
Copy link
Member Author

Hi @GerkinDev,

there is the proper error: https://stackblitz.com/edit/github-qdctmb?file=src%2Ftest.spec.ts

instead of asserting on the method of the ComponentFactoryResolver, we were asserting the spy, which of course hasn't been overridden.

satanTime added a commit that referenced this issue Jun 28, 2021
fix(core): allowing spies on ComponentFactoryResolver.resolveComponentFactory #736
@satanTime
Copy link
Member Author

v12.2.0 has been released and contains a fix for the issue. Feel free to reopen the issue or to submit a new one if you meet any problems.

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