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

Angular CT providers are not added to the TestingModule #23427

Closed
NicBright opened this issue Aug 18, 2022 · 10 comments
Closed

Angular CT providers are not added to the TestingModule #23427

NicBright opened this issue Aug 18, 2022 · 10 comments
Labels
CT Issue related to component testing

Comments

@NicBright
Copy link

NicBright commented Aug 18, 2022

Current behavior

The bug is obvious:

const { providers, ...configRest } = config
const componentFixture = createComponentFixture(component) as Type<T>
TestBed.configureTestingModule({
...bootstrapModule(componentFixture, configRest),
})
if (providers != null) {
TestBed.overrideComponent(componentFixture, {
add: {
providers,
},
})
}

The providers that are meant to be added to the TestingModule are instead only added in a call to TestBed.overrideComponent(). This way, it is not possible to alter dependencies of singleton services aka services that are { providedIn: 'root' } (or any custom module).

Desired behavior

Passed providers must be passed to TestBed.configureTestingModule()

Test code to reproduce

Irrelevant. See source code snippet above.

Cypress Version

10.6.0

Node version

irrelevant

Operating System

irrelevant

Debug Logs

No response

Other

No response

@NicBright
Copy link
Author

Until this bug is fixed, for those of you that stumble upon this. There's a workaround:

Create and add a dedicated "YourXyzTestingModule" to your imports. Within the definition of YourXyzTestingModule you can add providers which will be used as if you've added them to the providers config directly.

@mschile mschile added triage and removed triage labels Aug 18, 2022
@cypress-bot cypress-bot bot added the stage: investigating Someone from Cypress is looking into this label Aug 18, 2022
@mike-plummer
Copy link
Contributor

Hi @NicBright , we originally had the implementation you're asking for but we reverted it based on this issue. The component under test may need a custom implementation of a given provider but other uses rendered in that test may need another, and separating the providers in this way rather than forcing them to the TestBed level makes that a bit easier. For situations where you want a singleton provider your identified workaround (nice job finding that, btw) would be a solution. Of course, that may not be obvious to many users so this may be useful to note in our angular documentation

@jordanpowell88
Copy link
Contributor

@NicBright I am unable to duplicate the issue you are describing. I created an example of what you are describing here. Like @mike-plummer mentioned we are purposely overriding the component providers at the component level

@NicBright
Copy link
Author

NicBright commented Aug 24, 2022

@mike-plummer
Thanks for pointing me to that issue. As I see it yjaaidi described from his point of view how it should be without explaining why. Besides that, I can't see any further discussion in the thread.
Regardless, you mentioned:

The component under test may need a custom implementation of a given provider but other uses rendered in that test may need another

That's totally true! This could be addressed via #23425

@jordanpowell88
Refering to your example here
Of course you can still override the LoginService to inject a double inside an Angular CT this way for your component under test. However, you can neither inject a double for a child component (could be addressed via #23425), nor can you inject a double to be used as a dependency for any other singleton service. For example: how would you inject a double for HttpClient as it is required by LoginService?

To summarize:

  • Being able to place providers at the TestBed level is important to use doubles as dependencies for anything non-component (e.g. singleton services) - this issue
  • Being able to override providers for specific components (not only the component under test) is important to use doubles as dependencies for child-components - addressed by Angular CT should have proper support for overriding components #23425

@jordanpowell88
Copy link
Contributor

jordanpowell88 commented Aug 25, 2022

@NicBright I am still not able to reproduce the issue you are describing. Are you able to provide me a link to a repo? I just created the following test and it is working the way you are describing it would not:

  template: `
    <h1>Parent Component</h1>
    <app-child-providers></app-child-providers>
  `
})
export class ParentProvidersComponent {}`

@Injectable()
export class ChildProvidersService {
  constructor(private readonly http: HttpClient) {}

  getData(): Observable<string> {
    return this.http.get<string>('https://myfakeapiurl.com/api/data')
  }
}

@Component({
  selector: 'app-child-providers',
  template: `<button (click)="handleClick()">{{ message }}</button>`
})
export class ChildProvidersComponent {
  message = 'default message'

  constructor(private readonly service: ChildProvidersService) {}

  handleClick(): void {
    this.service.getData().pipe(
      take(1)
    ).subscribe(value => {
      this.message = value
    })
  }
}

it('can make test doubles for child components', () => {
    cy.mount(ParentProvidersComponent, {
      declarations: [ChildProvidersComponent],
      imports: [HttpClientModule],
      providers: [
        {
          provide: ChildProvidersService,
          useValue: {
            getData() {
              return of('test')
            }
          } as ChildProvidersService
        }
      ]
    })```

@NicBright
Copy link
Author

NicBright commented Aug 26, 2022

@jordanpowell88

The following example reproduces the two issues:

I've created an additional child component named AnotherChildProvidersComponent which differs from ChildProvidersComponent only in having a dedicated @Component({ providers }) setting.

@Component({
    template: `
        <h1>Parent Component</h1>
        <app-child-providers></app-child-providers>
    `,
})
export class ParentProvidersComponent {}

@Injectable({
    providedIn: 'root',
})
export class ChildProvidersService {
    constructor(private readonly http: HttpClient) {}

    getData(): Observable<string> {
        return this.http.get<string>('https://myfakeapiurl.com/api/data');
    }
}

@Component({
    selector: 'app-child-providers',
    template: `<button (click)="handleClick()">{{ message }}</button>`,
})
// eslint-disable-next-line rxjs-angular/prefer-takeuntil
export class ChildProvidersComponent {
    message = 'default message';

    constructor(private readonly service: ChildProvidersService) {}

    handleClick(): void {
        this.service
            .getData()
            .pipe(take(1))
            // eslint-disable-next-line rxjs-angular/prefer-takeuntil
            .subscribe((value) => {
                this.message = value;
            });
    }
}

@Component({
    selector: 'app-child-providers',
    template: `<button (click)="handleClick()">{{ message }}</button>`,
    providers: [ChildProvidersService], // such a provider cannot be replaced with a double!
})
// eslint-disable-next-line rxjs-angular/prefer-takeuntil
export class AnotherChildProvidersComponent {
    message = 'default message';

    constructor(private readonly service: ChildProvidersService) {}

    handleClick(): void {
        this.service
            .getData()
            .pipe(take(1))
            // eslint-disable-next-line rxjs-angular/prefer-takeuntil
            .subscribe((value) => {
                this.message = value;
            });
    }
}

describe('Cypress issue 23427', () => {
    it('This works - it seems that providers added to a component are also available for child components', () => {
        cy.mount(ParentProvidersComponent, {
            declarations: [ChildProvidersComponent],
            imports: [],
            providers: [
                {
                    provide: ChildProvidersService,
                    useValue: {
                        getData() {
                            return of('test');
                        },
                    } as ChildProvidersService,
                },
            ],
        });
        cy.contains('default message').click();
        cy.contains('test');
    });

    it('However they are NOT available for child components when they have dedicated @Component({providers}) configured (this test will fail)', () => {
        cy.mount(ParentProvidersComponent, {
            declarations: [AnotherChildProvidersComponent],
            imports: [HttpClientTestingModule],
            providers: [
                {
                    provide: ChildProvidersService,
                    useValue: {
                        getData() {
                            return of('test');
                        },
                    } as ChildProvidersService,
                },
            ],
        });
        cy.contains('default message').click();
        cy.contains('test');
    });

    it('You also cannot provide a double for a dependency of a singleton service (this test will fail)', () => {
        cy.mount(ParentProvidersComponent, {
            declarations: [ChildProvidersComponent],
            imports: [],
            providers: [
                { // try to provide a double for ChildProvidersService's dependency to HttpClient:
                    provide: HttpClient,
                    useValue: {
                        get() {
                            return of('test');
                        },
                    },
                },
            ],
        });
        cy.contains('default message').click();
        cy.contains('test');
    });
});

Result:
image

@jordanpowell88
Copy link
Contributor

jordanpowell88 commented Aug 26, 2022

Hey Nic. Thanks again for providing the thorough example of your problem.

The best way to accomplish what you are wanting to do at the moment is to use something like the following before your cy.mount() command:

TestBed.overrideProvider(ChildProvidersService, {
      useValue: {
        getMessage() {
          return of('test')
        }
      } as ChildProvidersService
    })

You still have access to the same Static TestBed we are using for mount so you should be able to overrideProviders that way. We may do some additional investigation into adding first party override support via #23425 but this should be sufficient work around for the time being

@ZachJW34
Copy link
Contributor

Going to reopen, as providers makes sense to map to the TestModule rather than the component since all of the other TestModuleMetadata properties are currently being mapped to the TestModule. We should add an API for overriding at the component level which could be relevant for normal components and Standalone components.

@ZachJW34 ZachJW34 reopened this Oct 25, 2022
@cypress-bot cypress-bot bot added stage: product backlog and removed stage: investigating Someone from Cypress is looking into this labels Oct 25, 2022
@nagash77 nagash77 added stage: investigating Someone from Cypress is looking into this routed-to-ct and removed stage: product backlog labels Oct 25, 2022
@astone123 astone123 removed the stage: investigating Someone from Cypress is looking into this label Oct 25, 2022
@rockindahizzy rockindahizzy added CT Issue related to component testing ct-core labels Oct 25, 2022
@jordanpowell88
Copy link
Contributor

This is solved in #24394

@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: sprint backlog labels Oct 26, 2022
@ZachJW34 ZachJW34 closed this as completed Nov 1, 2022
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 10, 2022

Released in 11.0.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v11.0.0, please open a new issue.

@cypress-bot cypress-bot bot removed the stage: needs review The PR code is done & tested, needs review label Nov 10, 2022
@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CT Issue related to component testing
Projects
None yet
Development

No branches or pull requests

8 participants