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

fix(@angular-devkit/build-angular): add a base href to karma debug context #19119

Merged

Conversation

IxquitilisSaid
Copy link
Contributor

This commits adds a base href value in the karma context iframe used to run unit tests.

It solves #19116 where a unit test throws:

No base href set. Please provide a value for the APP_BASE_HREF token or add a base element to the document.

Even if the application is fine. This is because the index.html from Angular CLI contains a base href value, but not the Karma context iframe. So when adding a unit test with a testing module that imports a NgModule, for example AppModule, which itself imports RouterModule, the unit test used to throw an error (regression appeared in router 3.1).

That could be solved by either adding RouterTestingModule to the testing module, or by adding a provider { provide: APP_BASE_HREF, useValue: '/' }, but required to understand the issue.

This solves the issue in a transparent way: developers won't even encounter the problem anymore.

@google-cla google-cla bot added the cla: yes label Oct 19, 2020
Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, for this @IxquitilisSaid, LGTM.

One small NIT, can you please add the PR description as part of the commit message body and add Closes #19116 as the commit message footer.

See: https://github.com/angular/angular-cli/blob/master/CONTRIBUTING.md#commit for more info.

@alan-agius4 alan-agius4 added target: patch This PR is targeted for the next patch release target: lts This PR is targeting a version currently in long-term support action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Oct 19, 2020
…ntext

This commits adds a base href value in the karma context iframe used to run unit tests where a unit test throws:
No base href set. Please provide a value for the APP_BASE_HREF token or add a base element to the document.
Even if the application is fine.
This is because the index.html from Angular CLI contains a base href value, but not the Karma context iframe.
So when adding a unit test with a testing module that imports a NgModule, for example AppModule,
which itself imports RouterModule, the unit test used to throw an error (regression appeared in router 3.1).
That could be solved by either adding `RouterTestingModule` to the testing module,
or by adding a provider `{ provide: APP_BASE_HREF, useValue: '/' }`, but required to understand the issue.
This solves the issue in a transparent way: developers won't even encounter the problem anymore.

Closes angular#19116
@IxquitilisSaid
Copy link
Contributor Author

@alan-agius4 I've updated the commit message, hopefully didn't mess it up.
Let me know if it's still wrong and I'll fix it again 😄

Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit message, looks good.

Thanks

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Oct 19, 2020
@alan-agius4
Copy link
Collaborator

@googlebot ping!

@alan-agius4 alan-agius4 merged commit ae94245 into angular:master Oct 19, 2020
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker target: lts This PR is targeting a version currently in long-term support target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[router/testing] No base href set errors in tests appear with router when using Debug Mode
2 participants