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/27 05:01:06 UTC
[GitHub] [cloudstack] Codegass opened a new pull request, #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Codegass opened a new pull request, #6676:
URL: https://github.com/apache/cloudstack/pull/6676
### Description
Fix #6659
This PR introduces Hamcrest API to simplify assertion in [`ImplicitPlannerTest.checkStrictModeWithCurrentAccountVmsPresent`](https://github.com/Codegass/cloudstack/blob/71056191f2bdad4be1a7eaf9bb73a7dcee3516f2/plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java#L199), [`ImplicitPlannerTest.checkStrictModeHostWithCurrentAccountVmsFull`](https://github.com/Codegass/cloudstack/blob/71056191f2bdad4be1a7eaf9bb73a7dcee3516f2/plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java#L233), and [`ImplicitPlannerTest.checkPreferredModePreferredHostAvailable`](https://github.com/Codegass/cloudstack/blob/71056191f2bdad4be1a7eaf9bb73a7dcee3516f2/plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java#L291).
Take `ImplicitPlannerTest.checkStrictModeWithCurrentAccountVmsPresent` as example:
Before Refactoring
```java
...
boolean foundNeededCluster = false;
for (Long cluster : clusterList) {
if (cluster != 1) {
fail("Found a cluster that shouldn't have been present, cluster id : " + cluster);
} else {
foundNeededCluster = true;
}
}
assertTrue("Didn't find cluster 1 in the list. It should have been present", foundNeededCluster);
...
```
After Refactoring
```java
import static org.hamcrest.Matchers.everyItem;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat; // org.junit.Assert.assertThat has been deprecated
...
assertThat("Found cluster that shouldn't have been present, only cluster 1 should have been present",
clusterList, everyItem(equalTo(1L)));
...
```
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [x] Minor
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
Local cluster environment (KVM, UBUNTU 18.04) runs the test suite.
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
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 pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
Codegass commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1230635851
Removed the unwanted reforamtting
--
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] boring-cyborg[bot] commented on pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1229124214
Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:
- In case of a new feature add useful documentation (raise doc PR at https://github.com/apache/cloudstack-documentation)
- Be patient and persistent. It might take some time to get a review or get the final approval from the committers.
- Pay attention to the quality of your code, ensure tests are passing and your PR doesn't have conflicts.
- Please follow [ASF Code of Conduct](https://github.com/apache/.github/blob/main/.github/CODE_OF_CONDUCT.md) for all communication including (but not limited to) comments on Pull Requests, Issues, Mailing list and Slack.
- Be sure to read the [CloudStack Coding Conventions](https://cwiki.apache.org/confluence/display/CLOUDSTACK/Coding+conventions).
Apache CloudStack is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@cloudstack.apache.org (https://cloudstack.apache.org/mailing-lists.html)
Slack: https://apachecloudstack.slack.com/
--
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 pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
Codegass commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1303664843
Hi guys, I just jump to ask if there is still something I need to improve to merge PR?
If so, please let me know. 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 a diff in pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#discussion_r957001455
##########
plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java:
##########
@@ -382,8 +360,8 @@ private void initializeForTest(VirtualMachineProfileImpl vmProfile, DataCenterDe
clustersWithEnoughCapacity.add(2L);
clustersWithEnoughCapacity.add(3L);
when(
- capacityDao.listClustersInZoneOrPodByHostCapacities(dataCenterId, 12L, noOfCpusInOffering * cpuSpeedInOffering, ramInOffering * 1024L * 1024L,
- Capacity.CAPACITY_TYPE_CPU, true)).thenReturn(clustersWithEnoughCapacity);
+ capacityDao.listClustersInZoneOrPodByHostCapacities(dataCenterId, 12L, noOfCpusInOffering * cpuSpeedInOffering, ramInOffering * 1024L * 1024L,
+ Capacity.CAPACITY_TYPE_CPU, true)).thenReturn(clustersWithEnoughCapacity);
Review Comment:
can you omit these reformats from your PR?
##########
plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java:
##########
@@ -606,4 +584,4 @@ public boolean match(MetadataReader mdr, MetadataReaderFactory arg1) throws IOEx
}
}
}
-}
+}
Review Comment:
can you omit these reformats from your PR?
##########
plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java:
##########
@@ -465,8 +443,8 @@ private void initializeForImplicitPlannerTest(boolean preferred) {
@Configuration
@ComponentScan(basePackageClasses = {ImplicitDedicationPlanner.class},
- includeFilters = {@Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)},
- useDefaultFilters = false)
+ includeFilters = {@Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)},
+ useDefaultFilters = false)
Review Comment:
can you omit these reformats from your 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
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1230641960
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6676 (SL-JID-2243)
--
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] blueorangutan commented on pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1242594054
UI build: :heavy_check_mark:
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6676 (SL-JID-2330)
--
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] sonarcloud[bot] commented on pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1242600946
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6676)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6676&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6676&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6676&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6676&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6676&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6676&metric=new_duplicated_lines_density&view=list)
--
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] codecov[bot] commented on pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1243546434
# [Codecov](https://codecov.io/gh/apache/cloudstack/pull/6676?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6676](https://codecov.io/gh/apache/cloudstack/pull/6676?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e8f168a) into [main](https://codecov.io/gh/apache/cloudstack/commit/c5262443595e4ed1bb9898654f3bed728331ffc0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c526244) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## main #6676 +/- ##
============================================
+ Coverage 5.87% 5.90% +0.02%
- Complexity 3934 3945 +11
============================================
Files 2454 2454
Lines 242683 242749 +66
Branches 37978 37991 +13
============================================
+ Hits 14261 14323 +62
+ Misses 226843 226842 -1
- Partials 1579 1584 +5
```
| [Impacted Files](https://codecov.io/gh/apache/cloudstack/pull/6676?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...dstack/storage/datastore/PrimaryDataStoreImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6676/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZW5naW5lL3N0b3JhZ2Uvdm9sdW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9jbG91ZHN0YWNrL3N0b3JhZ2UvZGF0YXN0b3JlL1ByaW1hcnlEYXRhU3RvcmVJbXBsLmphdmE=) | `2.25% <0.00%> (-0.09%)` | :arrow_down: |
| [...visor/vmware/manager/VmwareStorageManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6676/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy92bXdhcmUvc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci92bXdhcmUvbWFuYWdlci9WbXdhcmVTdG9yYWdlTWFuYWdlckltcGwuamF2YQ==) | `1.06% <0.00%> (-0.03%)` | :arrow_down: |
| [...ne/schema/src/main/java/com/cloud/host/HostVO.java](https://codecov.io/gh/apache/cloudstack/pull/6676/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9ob3N0L0hvc3RWTy5qYXZh) | `15.04% <0.00%> (ø)` | |
| [.../src/main/java/com/cloud/vm/UserVmManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6676/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3ZtL1VzZXJWbU1hbmFnZXJJbXBsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...chema/src/main/java/com/cloud/vm/VMInstanceVO.java](https://codecov.io/gh/apache/cloudstack/pull/6676/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC92bS9WTUluc3RhbmNlVk8uamF2YQ==) | `24.40% <0.00%> (ø)` | |
| [...n/java/com/cloud/resource/ResourceManagerImpl.java](https://codecov.io/gh/apache/cloudstack/pull/6676/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL3Jlc291cmNlL1Jlc291cmNlTWFuYWdlckltcGwuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...main/java/com/cloud/service/ServiceOfferingVO.java](https://codecov.io/gh/apache/cloudstack/pull/6676/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9zZXJ2aWNlL1NlcnZpY2VPZmZlcmluZ1ZPLmphdmE=) | `18.70% <0.00%> (ø)` | |
| [...oud/hypervisor/vmware/resource/VmwareResource.java](https://codecov.io/gh/apache/cloudstack/pull/6676/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy92bXdhcmUvc3JjL21haW4vamF2YS9jb20vY2xvdWQvaHlwZXJ2aXNvci92bXdhcmUvcmVzb3VyY2UvVm13YXJlUmVzb3VyY2UuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...esource/VmwareSecondaryStorageResourceHandler.java](https://codecov.io/gh/apache/cloudstack/pull/6676/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy92bXdhcmUvc3JjL21haW4vamF2YS9jb20vY2xvdWQvc3RvcmFnZS9yZXNvdXJjZS9WbXdhcmVTZWNvbmRhcnlTdG9yYWdlUmVzb3VyY2VIYW5kbGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...cloud/storage/resource/VmwareStorageProcessor.java](https://codecov.io/gh/apache/cloudstack/pull/6676/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGx1Z2lucy9oeXBlcnZpc29ycy92bXdhcmUvc3JjL21haW4vamF2YS9jb20vY2xvdWQvc3RvcmFnZS9yZXNvdXJjZS9WbXdhcmVTdG9yYWdlUHJvY2Vzc29yLmphdmE=) | `0.32% <0.00%> (+<0.01%)` | :arrow_up: |
| ... and [17 more](https://codecov.io/gh/apache/cloudstack/pull/6676/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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] acs-robot commented on pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1242593099
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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 pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1304215539
no @Codegass , merging
--
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 a diff in pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
Codegass commented on code in PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#discussion_r957511859
##########
plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java:
##########
@@ -382,8 +360,8 @@ private void initializeForTest(VirtualMachineProfileImpl vmProfile, DataCenterDe
clustersWithEnoughCapacity.add(2L);
clustersWithEnoughCapacity.add(3L);
when(
- capacityDao.listClustersInZoneOrPodByHostCapacities(dataCenterId, 12L, noOfCpusInOffering * cpuSpeedInOffering, ramInOffering * 1024L * 1024L,
- Capacity.CAPACITY_TYPE_CPU, true)).thenReturn(clustersWithEnoughCapacity);
+ capacityDao.listClustersInZoneOrPodByHostCapacities(dataCenterId, 12L, noOfCpusInOffering * cpuSpeedInOffering, ramInOffering * 1024L * 1024L,
+ Capacity.CAPACITY_TYPE_CPU, true)).thenReturn(clustersWithEnoughCapacity);
Review Comment:
Sure! I will reformat it today
--
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] blueorangutan commented on pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1242593143
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
--
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] sonarcloud[bot] commented on pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1229130178
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6676)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6676&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6676&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6676&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6676&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6676&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6676&metric=new_duplicated_lines_density&view=list)
--
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 a diff in pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
Codegass commented on code in PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#discussion_r957627850
##########
plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java:
##########
@@ -465,8 +443,8 @@ private void initializeForImplicitPlannerTest(boolean preferred) {
@Configuration
@ComponentScan(basePackageClasses = {ImplicitDedicationPlanner.class},
- includeFilters = {@Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)},
- useDefaultFilters = false)
+ includeFilters = {@Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)},
+ useDefaultFilters = false)
Review Comment:
Fixed🙌🏻
##########
plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java:
##########
@@ -606,4 +584,4 @@ public boolean match(MetadataReader mdr, MetadataReaderFactory arg1) throws IOEx
}
}
}
-}
+}
Review Comment:
Fixed🙌🏻
--
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] acs-robot commented on pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
acs-robot commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1230635584
Found UI changes, kicking a new UI QA build
@blueorangutan ui
--
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] blueorangutan commented on pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1230636014
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.
--
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] sonarcloud[bot] commented on pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1230703898
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_cloudstack&pullRequest=6676)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6676&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6676&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_cloudstack&pullRequest=6676&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_cloudstack&pullRequest=6676&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6676&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6676&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_cloudstack&pullRequest=6676&metric=new_duplicated_lines_density&view=list)
--
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 merged pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
DaanHoogland merged PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676
--
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] GutoVeronezi commented on a diff in pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on code in PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#discussion_r967125081
##########
plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java:
##########
@@ -248,15 +242,7 @@ public void checkStrictModeHostWithCurrentAccountVmsFull() throws InsufficientSe
// Check cluster 1 and 3 are not in the cluster list.
// Host 5 and 7 should also be in avoid list.
assertFalse("Cluster list should not be null/empty", (clusterList == null || clusterList.isEmpty()));
- boolean foundNeededCluster = false;
- for (Long cluster : clusterList) {
- if (cluster != 2) {
- fail("Found a cluster that shouldn't have been present, cluster id : " + cluster);
- } else {
- foundNeededCluster = true;
- }
- }
- assertTrue("Didn't find cluster 2 in the list. It should have been present", foundNeededCluster);
+ assertThat("Found cluster that shouldn't have been present, only cluster 2 should be present",clusterList, everyItem(equalTo(2L)));
Review Comment:
```suggestion
assertThat("Found cluster that shouldn't have been present, only cluster 2 should be present", clusterList, everyItem(equalTo(2L)));
```
##########
plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java:
##########
@@ -307,15 +293,7 @@ public void checkPreferredModePreferredHostAvailable() throws InsufficientServer
// Check cluster 1 and 2 are not in the cluster list.
// Host 5 and 6 should also be in avoid list.
assertFalse("Cluster list should not be null/empty", (clusterList == null || clusterList.isEmpty()));
- boolean foundNeededCluster = false;
- for (Long cluster : clusterList) {
- if (cluster != 3) {
- fail("Found a cluster that shouldn't have been present, cluster id : " + cluster);
- } else {
- foundNeededCluster = true;
- }
- }
- assertTrue("Didn't find cluster 3 in the list. It should have been present", foundNeededCluster);
+ assertThat("Found cluster that shouldn't have been present, only cluster 3 should be present",clusterList, everyItem(equalTo(3L)));
Review Comment:
```suggestion
assertThat("Found cluster that shouldn't have been present, only cluster 3 should be present", clusterList, everyItem(equalTo(3L)));
```
##########
plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java:
##########
@@ -211,15 +213,7 @@ public void checkStrictModeWithCurrentAccountVmsPresent() throws InsufficientSer
// Check cluster 2 and 3 are not in the cluster list.
// Host 6 and 7 should also be in avoid list.
assertFalse("Cluster list should not be null/empty", (clusterList == null || clusterList.isEmpty()));
- boolean foundNeededCluster = false;
- for (Long cluster : clusterList) {
- if (cluster != 1) {
- fail("Found a cluster that shouldn't have been present, cluster id : " + cluster);
- } else {
- foundNeededCluster = true;
- }
- }
- assertTrue("Didn't find cluster 1 in the list. It should have been present", foundNeededCluster);
+ assertThat("Found cluster that shouldn't have been present, only cluster 1 should be present",clusterList, everyItem(equalTo(1L)));
Review Comment:
```suggestion
assertThat("Found cluster that shouldn't have been present, only cluster 1 should be present", clusterList, everyItem(equalTo(1L)));
```
--
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 a diff in pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
Codegass commented on code in PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#discussion_r957627654
##########
plugins/deployment-planners/implicit-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java:
##########
@@ -382,8 +360,8 @@ private void initializeForTest(VirtualMachineProfileImpl vmProfile, DataCenterDe
clustersWithEnoughCapacity.add(2L);
clustersWithEnoughCapacity.add(3L);
when(
- capacityDao.listClustersInZoneOrPodByHostCapacities(dataCenterId, 12L, noOfCpusInOffering * cpuSpeedInOffering, ramInOffering * 1024L * 1024L,
- Capacity.CAPACITY_TYPE_CPU, true)).thenReturn(clustersWithEnoughCapacity);
+ capacityDao.listClustersInZoneOrPodByHostCapacities(dataCenterId, 12L, noOfCpusInOffering * cpuSpeedInOffering, ramInOffering * 1024L * 1024L,
+ Capacity.CAPACITY_TYPE_CPU, true)).thenReturn(clustersWithEnoughCapacity);
Review Comment:
Fixed🙌🏻
--
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 pull request #6676: Introducing Hamcrest API to simplify assertion in ImplicitPlannerTest
Posted by GitBox <gi...@apache.org>.
Codegass commented on PR #6676:
URL: https://github.com/apache/cloudstack/pull/6676#issuecomment-1232444044
It seems the Codecov failed due to some uploading issues. Please let me know if there any action I should take. 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