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(service-worker): add missing annotation for SwPush #19721

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@trotyl
Contributor

trotyl commented Oct 14, 2017

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #19525

What is the new behavior?

Service worker being available in JIT usage.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

This is just adding a static annotation, do we need integration test to cover it?

@gkalpak

The change LGTM. Not sure about testing. Wouldn't a simple unit test cover that? I think it would be nice to have some unit tests that access the services via DI anyway.

@trotyl

This comment has been minimized.

Show comment
Hide comment
@trotyl

trotyl Oct 14, 2017

Contributor

Added a test for DI access.

Contributor

trotyl commented Oct 14, 2017

Added a test for DI access.

{provide: NgswCommChannel, useValue: comm},
]
});
expect(() => TestBed.get(SwPush)).not.toThrow();

This comment has been minimized.

@gkalpak

gkalpak Oct 16, 2017

Member

Could you add a similar test for SwUpdate? (And while you are at it, maybe update the descriptions: describe('NgswPush'/'NgswUpdate' --> describe('SwPush'/'SwUpdate' 😁)

@gkalpak

gkalpak Oct 16, 2017

Member

Could you add a similar test for SwUpdate? (And while you are at it, maybe update the descriptions: describe('NgswPush'/'NgswUpdate' --> describe('SwPush'/'SwUpdate' 😁)

This comment has been minimized.

@trotyl

trotyl Oct 16, 2017

Contributor

Done~ 😃

@trotyl

trotyl Oct 16, 2017

Contributor

Done~ 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment