Add specs for auth.service

[jeroenheijmans]

At the start there were no tests involved in this sample. But since many people found the repository useful, and probably use mainly auth.service.ts as a starting point for their application, it seems useful to add a spec for the core thing they can copy/paste along.

I'm tagging this issue hacktoberfest/up-for-grabs, mainly because creating a start for all of this is something that can be picked up when you start out.

I prefer to have multiple PRs to do this in steps:

1. First PR to re-enable jasmine and test running for this repository, with just 1 it spec for auth.service.ts, that runs as part of the GitHub Actions pipelines (can be a dummy test)
2. Second PR to properly add a good mock for the OAuthService and then test the runInitialLoginSequence logic branches
3. Optionally a third PR to add some Jasmine Marbles based testing, to see what happens in various sequences

Let us know in the issue if you want to pick up any of these and at which timing, to prevent getting conflicts if multiple PRs come in.

(If no one feels like it that's also fine! Then I might add it myself, later. For now I commit to actively watching for incoming PRs.)

[aortiz24]

This is my first hacktoberfest and I would like to challenge myself.

Could I be assigned these three PRs?

Are you open to me asking questions if I get stuck while doing research?

Thanks!

[jeroenheijmans]

@aortiz24 Yeah sure thing! Don't forget to open the PRs after October starts if you want it to "count" for the DigitalOcean event of course.

You can reach me through various media, but most logical are probably either here in the issue, or you could join the contributing.today Hacktoberfest Discord (link topright of the website) and reach out there. You can already DM me there if you need help, and I'll see if they can add a channel for the repository.

Thanks for giving it a go!

[jeroenheijmans]

There's a sample-angular-oauth2-oidc-with-auth-guards channel in the contributing.today Discord where I'll be if anyone has hacktoberfest-related questions!

[aortiz24]

Hey @jeroenheijmans, I'm investigating what is needed to accomplish the first PR:

1.First PR to re-enable jasmine and test running for this repository, with just 1 it spec for auth.service.ts, that runs as part of the GitHub Actions pipelines (can be a dummy test).

It appears that I would need to follow the steps listed here: https://jasmine.github.io/setup/nodejs.html

If I'm on the right track, please share with me what you'd like me to test for. Thanks!

[jeroenheijmans]

@aortiz24 Using their howto does seem like the most thorough way to go about it. I'd guess it would be the harder but also more in-depth and insightful way to do things.

Personally, I'd go a different, probably easier (but less profound) way. If you generate a blanco project with ng new (using Angular CLI's latest version) they will scaffold everything that is needed to run unit/component tests in an Angular project. I'd recommend doing that and then comparing:

• angular.json, to see what contents is needed to run tests
• any files in the tree with called *.spec.* and see for which an equivalent is needed
• files generated for testing, from the top of my mind they'll include test.ts, karma.conf.js
• package.json's devDependencies (some are needed for testing) and scripts should probably get "test": "ng test" or some such

Try to run ng test whenever you feel it should be working, adding more missing pieces as you go, until it runs.

PS. It would be okay to start with a PR that allows running ng test locally, and we add GitHub Actions support later.

---

Feel free to open a PR even if you're halfway through and want some feedback. At work we tend to prefix the PR title with [WIP]: to indicate it is a "Work In Progress".

Ping me if you need more help!

[aortiz24]

I tried, but want to focus on other attainable pull requests. Thank you for giving me a chance!

[jeroenheijmans]

@aortiz24 No problem! Good luck with the other PRs, hope they provide you a better path into OSS than this issue did!

[valburyakov]

Hi @jeroenheijmans, mb do you want to try jest as test runner for testing instead of karma ? It's a more modern and faster tool than karma. I'm currently using It in my projects and It works like a charm. I would like to help you with jest.

[jeroenheijmans]

Hey @valburyakov! I knew about Jest but haven't had the time to try it out yet.

My main concern would be is that this sample (the repository) should be accessible to all kinds of Angular folks. I'd be afraid that folks using the default (Jasmine+Karma) testing setup would possibly be put off by a Jest setup, whereas the reverse would be less true (I expect those who explicitly choose Jest also know how to handle Jasmine tests).

Having said that, I'm not entirely opposed. In fact, it would be an opportunity for me to learn! I'd happily accept a PR that adds Jest-based tests for this repository. I might later decide to migrate that back to Jasmine if I feel it would hurt users with the "default" testing setup, so if you don't mind a chance that I might do that, then please do give it a go!

[valburyakov]

I got your point, makes sense. And as I see you have e2e tests which also use jasmine then It will be more consistent use jasmine for unit tests too.
But before implementing unit tests for auth-service need to make it more testable to provide via DI window and location objects in order to have ability to mock them.

[jeroenheijmans]

need to make it more testable to provide via DI window and location objects in order to have ability to mock them

Perfect! That'll be a great improvement. I'll have a peek at the PR.

Edit: I've tried the PR locally and it works as it should. Nice work! And again, I applaud changes to make things better testable, that'll make this sample all the more exemplary. 👍

[jeroenheijmans]

PR #67 was some more excellent work! <3

The most important thing we now have: a test setup so that

• A. People using this sample as a starter would have proper tests in their codebase.
• B. A setup that allows us to TDD additions and changes to this sample

Adding more tests now (more coverage in general, or Jasmine marbles tests, for fun) is in my opinion welcome-but-optional.

I'll leave the issue open for a moment. @valburyakov if you want to continue the hacktoberfest PR streak and add a follow-up, you'd be most welcome. If you feel it's all good for now, that's totally fine too (and then I'd probably close this issue, maybe create a fresh follow-up issue).

[valburyakov]

@jeroenheijmans Thx for feedback, glad to help. I think with auth-service we're done and you can close this issue.