-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
[Help Wanted] Increasing test coverage of Command classes #10796
Comments
@theoboldalex great, thanks! Maybe wait until I get a chance to review the first one to make sure you don't run off and do more of them with the wrong assumptions. I'll try to get to it soon. Also note the listing above is a bit outdated as I tried to work on this here and there when touching commands the last few months. But there is still plenty to be done :) |
No promises but I may make the occassional PR every now and then. |
I'd like to help out with this now and again - is there a way to see the current coverage? |
I've added tests for the BumpCommand to increase the test coverage. See composer#10796 Signed-off-by: Jesper Skytte <jesper@skytte.it>
@tomekpryjma nope sorry, gotta run the tests with |
I've added tests for the BumpCommand to increase the test coverage. See #10796 Signed-off-by: Jesper Skytte <jesper@skytte.it> Signed-off-by: Jesper Skytte <jesper@skytte.it>
Now at 41%! Here are the classes still needing the most effort IMO:
These need an install to be run to actual get a working vendor dir, using packages with type
|
Is this issue still relevant? |
@giulio-Joshi yes for sure, not much happened since my last comment above yours. |
Thanks for your answer. |
No it's just been 8 days, I'll get to it :) |
Putting my hand up to work on tests for PR created here: #11162. |
@Seldaek What kind of skill/talent/tools are required/useful to contribute with testing? |
@siims-biz knowledge of PHPUnit, some knowledge of composer (from user perspective good, internals better), the easy ones have been covered already though so if you aren't super familiar with all this I would say maybe rather find something else to work on, but up to you :) |
Now at 52% |
I'm going to make a start on testing |
54.4% now, thanks everyone involved so far! Here are the classes still needing the most effort, but overall we are really in a much better spot now I would say with the most critical commands pretty well covered:
These need an install to be run to actual get a working vendor dir, using packages with type
|
Will take test cases for commands |
Hi @Seldaek, I'd love to continue working on the above checklist, but I'm still waiting for Tests for base dependency command #11547 feedback, once I've received first feedback I could replicate that same approach in my upcoming PRs, thanks |
Will try to complete |
FYI - I'm writing a lot of test for ShowCommand :) |
As you can see on this image, Command classes have very poor coverage currently (16% of lines covered in total):
Edit: To see latest guidelines on what to work on see #10796 (comment)
Here some examples of how to write sane integration tests for most commands:
composer/tests/Composer/Test/Command/RunScriptCommandTest.php
Lines 92 to 113 in 176d258
composer/tests/Composer/Test/Command/ConfigCommandTest.php
Lines 23 to 52 in 176d258
composer/tests/Composer/Test/Command/BumpCommandTest.php
Lines 28 to 47 in 70f2dd6
If you feel like helping, PRs are very much welcome, I don't think we absolutely need 100% coverage but having the most common use cases covered would already be very valuable. Please target the
main
branch if you want to contribute some, and feel free to announce it here if you start some larger chunk of work to avoid duplicating efforts.The text was updated successfully, but these errors were encountered: