-
-
Notifications
You must be signed in to change notification settings - Fork 167
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
Some unit test improvements #6220
Conversation
About the files in the About coverage in general going down: I'd say if the new annotation is correct (in that it declares the method(s) that are actually being tested with assertions), then it's fine if coverage goes down as it helps us to see areas of the code that are not properly tested. Even better would of course be to add tests for those now uncovered methods. I think we need to keep in mind that the coverage percentage is also relevant for marketing (e.g. we use it on the "for developers" page). What I wouldn't do is to add |
The problem is that often those test for config or other non-class code also touch a dozen other classes etc - so without |
That is true. But I feel we need a solution in PHPUnit then to be able to do it properly. Before we have that, I'd rather exclude the whole config dir from the coverage allowlist. But that's also not great as a lot of our logic happens there that we need to test. So TBH I feel like keeping the status quo is overall the best option. |
A hack could be to put all these config tests into a separate |
581d335
to
192d59f
Compare
3b1cca0
to
003f240
Compare
003f240
to
1fb7ab2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have not reviewed all changes in detail, but skimmed over the general setup changes. Good idea to split up the KirbyTags tests into separate files. Also great that many of the previously uncovered methods now have tests. 👍
Could you please add/update the first post with the final contents of this PR?
Fixes
null
value forText\KirbyTag::parent()
Housekeeping
Options
,Query
,Text
and partiallyToolkit
packages tests, including additional tests for uncovered code