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

Cleanup ng test #59

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Cleanup ng test #59

wants to merge 6 commits into from

Conversation

Xiphoseer
Copy link
Collaborator

Angular generates test cases with every component template, but we haven't actually run those up until now. This branch adds the common modules to the test fixtures in the majority of places. The idea is to gate on ng test running in the future using GH actions.

// cc: @lcdr

@github-actions
Copy link

github-actions bot commented Sep 1, 2021

Visit the preview URL for this PR (updated for commit e8de0a6):

https://lu-explorer--pr59-tests-s0lvz6cr.web.app

(expires Sun, 12 Sep 2021 11:01:24 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@lcdr
Copy link
Collaborator

lcdr commented Sep 3, 2021

Getting tests working and integrated with CI is definitely a good idea. I have no experience with Angular testing, so I'll trust you that the changes are reasonable, feel free to merge.

@Xiphoseer
Copy link
Collaborator Author

I guess the main question is whether it makes sense to need to specify the "global" service modules and pipes that are loaded by app.module.ts in production for every component that uses them individually, which is what this PR is doing.

I think there'd be a way to register e.g. LuJsonService for all components by default and override it for mocks, but it may also catch more bugs to do it case by case.

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

Successfully merging this pull request may close these issues.

2 participants