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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test framework specific warnings #193

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

Conversation

SkyaTura
Copy link
Collaborator

@SkyaTura SkyaTura commented Apr 4, 2021

Hey @Shinigami92 , I found a (kind of) easy way to test framework warnings. Could you please have a look on this?

By the way, I'm not a TypeScripter myself, so there is some lint errors that I don't know how to fix.

Also, I don't know how to trigger Angular's specific warnings 馃槀

@SkyaTura
Copy link
Collaborator Author

SkyaTura commented Apr 4, 2021

I created this from the add-framework-option branch, I didn't noticed that you've merged that branch until I pushed this.

@Shinigami92
Copy link
Member

I created this from the add-framework-option branch, I didn't noticed that you've merged that branch until I pushed this.

Yeah it was in this moment ^^

I think you can use git rebase -i main to "fix" this branch
And the git push -f

@Shinigami92
Copy link
Member

@SkyaTura Or maybe if this doesn't work well, you need git rebase --onto. But for this you should read documentation, cause I don't know exactly myself how it works.

@SkyaTura
Copy link
Collaborator Author

SkyaTura commented Apr 4, 2021

Merge main into the branch works just fine to "fix" it. Although it keeps a lot a commits 馃槀

@Shinigami92
Copy link
Member

Yeah with the rebase the commits will go away, you should try it

But for testing the logs we should consider using mocks or spys (https://stackoverflow.com/questions/49096093/how-do-i-test-a-jest-console-log) instead of adding runtime code to the project 馃

@Shinigami92 Shinigami92 force-pushed the add-framework-option-test-warnings branch from 258d62b to 56544e3 Compare April 7, 2021 14:37
@Shinigami92
Copy link
Member

So I now used git rebase --onto main add-framework-option
You may need to re-checkout the branch on your local machine. Maybe use git branch -D add-framework-option-test-warnings to delete it locally, and then re-checkout.

@Shinigami92
Copy link
Member

@SkyaTura I'm looking forward to release a v1.14.0 bundling this feature, the new docs and hopefully also now issue-167 if h0merjam will tackle it 馃槂
Do you have time for addressing and working on the console-mock variant in the next time (maybe over upcoming weekend)?

@SkyaTura
Copy link
Collaborator Author

SkyaTura commented Apr 8, 2021

I can work on it in this weekend, for sure.

@SkyaTura
Copy link
Collaborator Author

SkyaTura commented Apr 9, 2021

Hey @Shinigami92 . 馃憢馃徎

I successfully managed to use jest.spyOn to avoid adding runtime code, however, I still don't know what to do to fix those lint errors. 馃槹

Also, I need someone that actually works with Angular to tell me test cases that throws those warnings, because I wasn't able to reproduce it.

@SkyaTura SkyaTura added framework: Angular Related to the framework Angular help wanted We are looking for community help labels Apr 9, 2021
@Shinigami92
Copy link
Member

Don't fear the linting warnings and errors 馃槃
I can fix them in the evening today

You may look into some older issues like these:

https://github.com/prettier/plugin-pug/issues?q=is%3Aissue+label%3A%22framework%3A+Angular%22
https://github.com/prettier/plugin-pug/issues?q=is%3Aissue+log

@SkyaTura
Copy link
Collaborator Author

I managed to add one test case, unfortunately I didn't found examples to the other warnings 馃槥

@Shinigami92
Copy link
Member

I managed to add one test case, unfortunately I didn't found examples to the other warnings

Hey, think positive 馃槂
-> You already found one console warning message and that is a good start

I think most of the messages where catched due to reports from using Vue
So there where hints to e.g. change pug code to use improved workarounds and that way make your code even better

But I could be false remembering here

I think I will also look into some messages again the next few days and find out how to throw them forcefully in tests

@Shinigami92
Copy link
Member

WOW! @SkyaTura I found out that most tests are falling falsely into the default case 馃槙
So I think I need to investigate more into it later on what is needed and if the if-cases are not up to date anymore or so 馃

But I will do that later, give me some days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
framework: Angular Related to the framework Angular help wanted We are looking for community help
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants