You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/08/22 19:05:42 UTC
[GitHub] [cloudstack] Codegass opened a new issue, #6666: Assert Missing in Test Case testCreateSuccess
Codegass opened a new issue, #6666:
URL: https://github.com/apache/cloudstack/issues/6666
##### ISSUE TYPE
* Improvement Request
##### COMPONENT NAME
unit-test
##### OS / ENVIRONMENT
N/A
##### SUMMARY
I noticed that [ testCreateSuccess ](https://github.com/apache/cloudstack/blob/6d11e2faa99a27bcb3b124f30a87748c91871514/api/src/test/java/org/apache/cloudstack/api/command/test/ScaleVMCmdTest.java#L66) does not contain assert statements. This is confusing to me since I do not know whether this test case pass or fail. In my understanding, `testCreateSuccess` is testing `scaleVMCmd.execution()` function to make sure the VM is created successfully. But it just ends there, not checking any of the running results.
The `scaleVMCmd.execution()` function has defined multiple exceptions and mocked all the values getting functions, but the test case does not have `throw Exception` or `try-catch` structure for the `scaleVMCmd.execution()` . Also, how many times the mocked functions are called are still not checked.
<!--
When I am exploring the source code of the project, I noticed that there is a test case that doesn’t have a proper Assertion at the end of it:
* [ testCreateSuccess ](https://github.com/apache/cloudstack/tree/main/src/test/java/org/apache/cloudstack/api/command/test/ScaleVMCmdTest.java#L66) is testing `scaleVMCmd.execution()` function to make sure the VM is created successfully. But it just ends there, not checking any of the running results.
This case really confused me, I noticed that the `scaleVMCmd.execution()` function has defined multiple exceptions and mocked all the values getting functions, but how many times the mocked functions are called are still not checked.
-->
I would suggest refactoring the cases by adding execution time verifications with Mockito. The goal is to make the test condition more explicit and reusable.
if you think this makes sense, I would be more than happy to try the refactoring and submit a PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on issue #6666: Assert Missing in Test Case testCreateSuccess
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on issue #6666:
URL: https://github.com/apache/cloudstack/issues/6666#issuecomment-1227301131
@Codegass thanks for the issue - are you going to work on them? You can directly raise pull requests for your ideas, no need to open issues if you've any pull requests in coming.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland closed issue #6666: Assert Missing in Test Case testCreateSuccess
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland closed issue #6666: Assert Missing in Test Case testCreateSuccess
URL: https://github.com/apache/cloudstack/issues/6666
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on issue #6666: Assert Missing in Test Case testCreateSuccess
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on issue #6666:
URL: https://github.com/apache/cloudstack/issues/6666#issuecomment-1603861851
@Codegass I am closing this issue. If you feel this is invalid please reopen or create a new one
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on issue #6666: Assert Missing in Test Case testCreateSuccess
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on issue #6666:
URL: https://github.com/apache/cloudstack/issues/6666#issuecomment-1537904923
@Codegass we had agreed in the community, that an explicit issue isn't required if an author is ready with the PR. For support building, we encourage community to start a `[DISCUSS]` thread on the community dev@ ML https://cloudstack.apache.org/mailing-lists.html
Is this issue still relevant or could be closed? Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on issue #6666: Assert Missing in Test Case testCreateSuccess
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #6666:
URL: https://github.com/apache/cloudstack/issues/6666#issuecomment-1223650863
@Codegass I think you can leave this. if the creation is not successful it would have thrown an exception. On the other hand, this is one that is tested implicitely a lot of times as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Codegass commented on issue #6666: Assert Missing in Test Case testCreateSuccess
Posted by GitBox <gi...@apache.org>.
Codegass commented on issue #6666:
URL: https://github.com/apache/cloudstack/issues/6666#issuecomment-1227921992
@rohityadavcloud Thank you for the reply! I used to think submitting the PR directly is too strong and may disturbing the community. After read you reply, I realized that PR is a capable option for this kind of suggestions.
I will try to submit the PR this week.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org