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/26 02:42:10 UTC
[GitHub] [cloudstack] chenhao-work opened a new issue, #6674: Refactor `QemuImgTest.testCreateAndInfo()` to avoid the "Eager Test"
chenhao-work opened a new issue, #6674:
URL: https://github.com/apache/cloudstack/issues/6674
##### ISSUE TYPE
* Improvement Request
##### COMPONENT NAME
unit-test
##### OS / ENVIRONMENT
N/A
##### SUMMARY
I noticed that all the test cases in the the [`QemuImgTest`](https://github.com/apache/cloudstack/blob/f23a4db6d2658781519c8820b03a2ad153df1024/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java) are having `qemu.create()` with one another function as the testing targets. Here I will use the [`testCreateAndInfoWithOptions`](https://github.com/apache/cloudstack/blob/f23a4db6d2658781519c8820b03a2ad153df1024/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java#L67-L94) case as example.
```java
...
QemuImgFile file = new QemuImgFile(filename, size, PhysicalDiskFormat.QCOW2);
String clusterSize = "131072";
Map<String, String> options = new HashMap<String, String>();
options.put("cluster_size", clusterSize);
QemuImg qemu = new QemuImg(0);
qemu.create(file, options);
Map<String, String> info = qemu.info(file);
...
```
The method `qemu.create(file, options)` and method `qemu.info(file)` are the two testing targets of the test case, which makes the testing goal of the test case obscure. And when the first method `qemu.create(file, options)` fails, the the second method will not be tested in the CI\CD process. It will take developer sometime to make sure which method is the actual buggy one.
So I suggest split the test case into two separate case:
1. testCreateWithOptions: Extract the [line 68 to 79](https://github.com/apache/cloudstack/blob/f23a4db6d2658781519c8820b03a2ad153df1024/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java#L68-L79) and add the file existing check `assertTrue(file.exist())` in the end as the test case to test `qemu.create(file, options)` .
2. testInfoWithOptions: Extract the line [line 80 to line 89](https://github.com/apache/cloudstack/blob/f23a4db6d2658781519c8820b03a2ad153df1024/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java#L80-L89) and add the sequence of methods related to `qemu.create(file, options)` in the beginning to create the file with option as the test case to test `qemu.info (file)`. Here the `qemu.create(file, options)` is a test setup.3.
By Splitting the test case, the checking for the `qemu.create(file, options)` will be more detailed and make the testing target more clear and easy to locate the issue by the test case name. This will also improve the readability of the test cases, making developers easy to understand the test target from the test case name.
###### Operational impact
The testing refactoring won’t affect any part of the production code behavior. Also, only the original cases are replaced with unit tests.
###### NOTE
Please let me know if you think this proposal makes sense, If so I can submit a PR accordingly, If not, comments are appreciated.
--
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] DaanHoogland commented on issue #6674: Refactor `QemuImgTest.testCreateAndInfo()` to avoid the "Eager Test"
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #6674:
URL: https://github.com/apache/cloudstack/issues/6674#issuecomment-1228633540
@chenhao-work go ahead, as you say this won't affect production code and if the cleanup is good it is a nice enhancement to the QA of cloudstack. One concern is that if one test uses the functionality of the other as a prerequisite, and the order of execution is not controlled, the possibility that the create() fails in the info() test is still there. it would be better if the create can be mocked somehow and the info executed on a mock. In this case that might not be possible, or hard to achieve. a possibility would be to add a mock-file to test against.
please go ahead and create your PR, looking forward to it.
--
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
Re: [I] Refactor `QemuImgTest.testCreateAndInfo()` to avoid the "Eager Test" [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud commented on issue #6674:
URL: https://github.com/apache/cloudstack/issues/6674#issuecomment-2084857484
Thanks @chenhao-work for the issue and suggestion. For the proposed changes you may go ahead and propose a PR, an issue isn't always needed or necessary for proposing a new PR. For now, I'm closing this issue as this isn't a bug.
--
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
Re: [I] Refactor `QemuImgTest.testCreateAndInfo()` to avoid the "Eager Test" [cloudstack]
Posted by "rohityadavcloud (via GitHub)" <gi...@apache.org>.
rohityadavcloud closed issue #6674: Refactor `QemuImgTest.testCreateAndInfo()` to avoid the "Eager Test"
URL: https://github.com/apache/cloudstack/issues/6674
--
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