You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by alexandrelimassantana <gi...@git.apache.org> on 2016/03/19 20:43:28 UTC
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
GitHub user alexandrelimassantana opened a pull request:
https://github.com/apache/cloudstack/pull/1445
Fixed Profiler's unit tests bugs.
Problem:
The TestProfiler class was using Java Thread methods to test the
Profiler's functionality. That was causing the tests to fail sometimes
since the JVM's thread priority could be low on some OS.
Fix:
Using PowerMockito to mock the System calls, the threads could be
removed. This makes the tests considerably faster, OS independent and
still guarantees the correct implementation of the Profiler class.
The changes on the Profiler's class was only to shorten the class's line
size by not assigning the return value to a variable returning it
straight out.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rafaelweingartner/cloudstack lrg-cs-hackday-025
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1445.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1445
----
commit 8d6370b21fd527dc0046c0a1d67dce35dbf15681
Author: Alexandre Limas Santana <al...@gmail.com>
Date: 2016-03-19T19:25:47Z
Fixed Profiler's unit tests bugs.
Problem:
The TestProfiler class was using Java Thread methods to test the
Profiler's functionality. That was causing the tests to fail sometimes
since the JVM's thread priority could be low on some OS.
Fix:
Using PowerMockito to mock the System calls, the threads could be
removed. This makes the tests considerably faster, OS independent and
still guarantees the correct implementation of the Profiler class.
The changes on the Profiler's class was only to shorten the class's line
size by not assigning the return value to a variable returning it
straight out.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209028959
I appreciate it. :+1:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209020807
@pdube we need to run CI against it first, but otherwise, yes it is ready...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209024302
I did nto noticed that @swill LGTM.
So, it is OK.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209021841
I would say it is missing an LGTM.
But, it is very good the code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-199858341
Built and ran the unit tests. LGTM
[INFO] Apache CloudStack Developer Tools - Checkstyle Configuration SUCCESS [ 1.106 s]
[INFO] Apache CloudStack ................................. SUCCESS [ 0.975 s]
[INFO] Apache CloudStack Maven Conventions Parent ........ SUCCESS [ 1.918 s]
[INFO] Apache CloudStack Framework - Managed Context ..... SUCCESS [ 1.485 s]
[INFO] Apache CloudStack Utils ........................... SUCCESS [ 11.555 s]
[INFO] Apache CloudStack Framework ....................... SUCCESS [ 0.047 s]
[INFO] Apache CloudStack Framework - Event Notification .. SUCCESS [ 1.561 s]
[INFO] Apache CloudStack Framework - Configuration ....... SUCCESS [ 1.210 s]
[INFO] Apache CloudStack API ............................. SUCCESS [ 12.870 s]
[INFO] Apache CloudStack Framework - REST ................ SUCCESS [ 0.644 s]
[INFO] Apache CloudStack Framework - IPC ................. SUCCESS [ 0.640 s]
[INFO] Apache CloudStack Cloud Engine .................... SUCCESS [ 0.030 s]
[INFO] Apache CloudStack Cloud Engine API ................ SUCCESS [ 3.529 s]
[INFO] Apache CloudStack Framework - Security ............ SUCCESS [ 0.499 s]
[INFO] Apache CloudStack Core ............................ SUCCESS [ 4.886 s]
[INFO] Apache CloudStack Agents .......................... SUCCESS [ 1.546 s]
[INFO] Apache CloudStack Framework - Clustering .......... SUCCESS [ 1.026 s]
[INFO] Apache CloudStack Framework - Event Notification .. SUCCESS [ 0.332 s]
[INFO] Apache CloudStack Cloud Engine Schema Component ... SUCCESS [ 10.903 s]
[INFO] Apache CloudStack Framework - Jobs ................ SUCCESS [ 2.049 s]
[INFO] Apache CloudStack Cloud Engine Internal Components API SUCCESS [ 0.551 s]
[INFO] Apache CloudStack Server .......................... SUCCESS [ 25.862 s]
[INFO] Apache CloudStack Framework - Quota ............... SUCCESS [ 1.987 s]
[INFO] Apache CloudStack Usage Server .................... SUCCESS [ 1.694 s]
[INFO] Apache CloudStack Cloud Engine Orchestration Component SUCCESS [ 0.648 s]
[INFO] Apache CloudStack Cloud Services .................. SUCCESS [ 0.016 s]
[INFO] Apache CloudStack Secondary Storage ............... SUCCESS [ 0.125 s]
[INFO] Apache CloudStack Secondary Storage Service ....... SUCCESS [ 0.678 s]
[INFO] Apache CloudStack Engine Storage Component ........ SUCCESS [ 5.129 s]
[INFO] Apache CloudStack Engine Storage Volume Component . SUCCESS [ 0.588 s]
[INFO] Apache CloudStack Engine Storage Image Component .. SUCCESS [ 0.472 s]
[INFO] Apache CloudStack Engine Storage Data Motion Component SUCCESS [ 0.592 s]
[INFO] Apache CloudStack Engine Storage Cache Component .. SUCCESS [ 0.545 s]
[INFO] Apache CloudStack Engine Storage Snapshot Component SUCCESS [ 3.074 s]
[INFO] Apache CloudStack Cloud Engine API ................ SUCCESS [ 0.567 s]
[INFO] Apache CloudStack Cloud Engine Service ............ SUCCESS [ 0.596 s]
[INFO] Apache CloudStack Plugin POM ...................... SUCCESS [ 0.266 s]
[INFO] Apache CloudStack Plugin - API Rate Limit ......... SUCCESS [ 3.276 s]
[INFO] Apache CloudStack Plugin - Storage Volume default provider SUCCESS [ 0.485 s]
[INFO] Apache CloudStack Plugin - Storage Volume SolidFire Provider SUCCESS [ 0.574 s]
[INFO] Apache CloudStack Plugin - API SolidFire .......... SUCCESS [ 0.483 s]
[INFO] Apache CloudStack Plugin - API Discovery .......... SUCCESS [ 1.377 s]
[INFO] Apache CloudStack Plugin - ACL Static Role Based .. SUCCESS [ 0.497 s]
[INFO] Apache CloudStack Plugin - Host Anti-Affinity Processor SUCCESS [ 0.428 s]
[INFO] Apache CloudStack Plugin - Explicit Dedication Processor SUCCESS [ 0.404 s]
[INFO] Apache CloudStack Plugin - User Concentrated Pod Deployment Planner SUCCESS [ 0.429 s]
[INFO] Apache CloudStack Plugin - User Dispersing Deployment Planner SUCCESS [ 0.424 s]
[INFO] Apache CloudStack Plugin - Implicit Dedication Planner SUCCESS [ 2.820 s]
[INFO] Apache CloudStack Plugin - Skip Heurestics Planner SUCCESS [ 0.635 s]
[INFO] Apache CloudStack Plugin - Host Allocator Random .. SUCCESS [ 0.483 s]
[INFO] Apache CloudStack Plugin - Dedicated Resources .... SUCCESS [ 2.748 s]
[INFO] Apache CloudStack Plugin - Hypervisor OracleVM .... SUCCESS [ 0.560 s]
[INFO] Apache CloudStack Plugin - Open vSwitch ........... SUCCESS [ 0.566 s]
[INFO] Apache CloudStack Plugin - Hypervisor XenServer ... SUCCESS [ 21.803 s]
[INFO] Apache CloudStack Plugin - Hypervisor KVM ......... SUCCESS [ 3.487 s]
[INFO] Apache CloudStack Plugin - RabbitMQ Event Bus ..... SUCCESS [ 0.526 s]
[INFO] Apache CloudStack Plugin - In Memory Event Bus .... SUCCESS [ 1.354 s]
[INFO] Apache CloudStack Plugin - Kafka Event Bus ........ SUCCESS [ 0.396 s]
[INFO] Apache CloudStack Plugin - Hypervisor Baremetal ... SUCCESS [ 0.454 s]
[INFO] Apache CloudStack Plugin - Hypervisor UCS ......... SUCCESS [ 0.444 s]
[INFO] Apache CloudStack Plugin - Hypervisor Hyper-V ..... SUCCESS [ 0.792 s]
[INFO] Apache CloudStack Plugin - Hypervisor OracleVM3 ... SUCCESS [ 17.310 s]
[INFO] Apache CloudStack Plugin - Network Elastic Load Balancer SUCCESS [ 1.753 s]
[INFO] Apache CloudStack Plugin - Network Internal Load Balancer SUCCESS [ 4.308 s]
[INFO] Apache CloudStack Framework - Spring Life Cycle ... SUCCESS [ 0.280 s]
[INFO] Apache CloudStack Plugin - Network Juniper Contrail SUCCESS [ 5.478 s]
[INFO] Apache CloudStack Plugin - Palo Alto .............. SUCCESS [ 2.308 s]
[INFO] Apache CloudStack Plugin - Network Netscaler ...... SUCCESS [ 0.555 s]
[INFO] Apache CloudStack Plugin - Network Nicira NVP ..... SUCCESS [ 5.086 s]
[INFO] Apache CloudStack Plugin - BigSwitch Virtual Network Segment SUCCESS [ 2.470 s]
[INFO] Apache CloudStack Plugin - Network Brocade VCS .... SUCCESS [ 2.579 s]
[INFO] Apache CloudStack Plugin - Midokura Midonet ....... SUCCESS [ 1.744 s]
[INFO] Apache CloudStack Plugin - Stratosphere SSP ....... SUCCESS [ 2.396 s]
[INFO] Apache CloudStack Plugin - Network Opendaylight ... SUCCESS [ 1.376 s]
[INFO] Apache CloudStack Plugin - Storage Allocator Random SUCCESS [ 0.473 s]
[INFO] Apache CloudStack Plugin - User Authenticator LDAP SUCCESS [ 1.030 s]
[INFO] Apache CloudStack Plugin - User Authenticator MD5 . SUCCESS [ 1.579 s]
[INFO] Apache CloudStack Plugin - User Authenticator PBKDF2-SHA-256 SUCCESS [ 3.007 s]
[INFO] Apache CloudStack Plugin - User Authenticator Plain Text SUCCESS [ 0.546 s]
[INFO] Apache CloudStack Plugin - User Authenticator SAML2 SUCCESS [ 18.772 s]
[INFO] Apache CloudStack Plugin - User Authenticator SHA256 Salted SUCCESS [ 1.376 s]
[INFO] Apache CloudStack Plugin - Dns Notifier Example ... SUCCESS [ 0.438 s]
[INFO] Apache CloudStack Plugin - Storage Image S3 provider SUCCESS [ 0.447 s]
[INFO] Apache CloudStack Plugin - Storage Image Swift provider SUCCESS [ 0.451 s]
[INFO] Apache CloudStack Plugin - Storage Image default provider SUCCESS [ 0.440 s]
[INFO] Apache CloudStack Plugin - Storage Image sample provider SUCCESS [ 0.488 s]
[INFO] Apache CloudStack Plugin - Storage Volume Nexenta Provider SUCCESS [ 0.570 s]
[INFO] Apache CloudStack Plugin - Storage Volume CloudByte Provider SUCCESS [ 0.536 s]
[INFO] Apache CloudStack Plugin - Storage Volume sample provider SUCCESS [ 0.512 s]
[INFO] Apache CloudStack Plugin - SNMP Alerts ............ SUCCESS [ 1.302 s]
[INFO] Apache CloudStack Plugin - Syslog Alerts .......... SUCCESS [ 0.969 s]
[INFO] Apache CloudStack Plugin - Network VXLAN .......... SUCCESS [ 1.547 s]
[INFO] Apache CloudStack Plugin - GloboDNS ............... SUCCESS [ 3.733 s]
[INFO] Apache CloudStack Plugin - Quota Service .......... SUCCESS [ 2.127 s]
[INFO] Apache CloudStack Framework - Spring Module ....... SUCCESS [ 2.605 s]
[INFO] Apache CloudStack Secondary Storage Controller .... SUCCESS [ 0.376 s]
[INFO] Apache CloudStack Client UI ....................... SUCCESS [ 1.352 s]
[INFO] Apache CloudStack Console Proxy - RDP Client ...... SUCCESS [ 5.639 s]
[INFO] Apache CloudStack Console Proxy ................... SUCCESS [ 0.122 s]
[INFO] Apache CloudStack Console Proxy - Server .......... SUCCESS [ 0.729 s]
[INFO] Apache CloudStack Framework - QuickCloud .......... SUCCESS [ 0.112 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 04:08 min
[INFO] Finished at: 2016-03-22T11:04:28-05:00
[INFO] Final Memory: 94M/763M
[INFO] ------------------------------------------------------------------------
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209025189
@pdube sorry, my brain is into something else right now and I didn't review the scope of the code. Yes, your tests are valid tests for this change. So yes, I will add this to my queue to be merged. Thx guys...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-199535142
This LGTM. @pdube, would you mind reviewing this for me?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1445#discussion_r59422694
--- Diff: utils/src/test/java/com/cloud/utils/TestProfiler.java ---
@@ -19,84 +19,79 @@
package com.cloud.utils;
-import org.apache.log4j.Logger;
import org.junit.Assert;
+import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
import com.cloud.utils.testcase.Log4jEnabledTestCase;
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({Profiler.class})
public class TestProfiler extends Log4jEnabledTestCase {
- protected final static Logger s_logger = Logger.getLogger(TestProfiler.class);
- private static final long MILLIS_FACTOR = 1000l;
- private static final int MARGIN = 100;
- // Profiler uses System.nanoTime which is not reliable on the millisecond
- // A sleep of 1000 milliseconds CAN be measured as 999 milliseconds
- // Therefore make the second larger, by 10% of the margin
- private static final long ONE_SECOND = 1000l + (MARGIN / 10);
- private static final double EXPONENT = 3d;
+ private static final long SLEEP_TIME_NANO = 1000000000L;
+ private static Profiler pf;
+
+ @Before
+ public void setUp() {
+ pf = new Profiler();
+ PowerMockito.mockStatic(System.class);
+ PowerMockito.when(System.nanoTime()).thenReturn(0L, SLEEP_TIME_NANO);
+ }
@Test
public void testProfilerInMillis() {
- s_logger.info("testProfiler() started");
+ //Given
--- End diff --
@alexandrelimassantana these comments "//Given", "//When", and so forth could be removed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209017286
@swill this looks ready, no?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1445#discussion_r56813778
--- Diff: utils/src/test/java/com/cloud/utils/TestProfiler.java ---
@@ -19,84 +19,79 @@
package com.cloud.utils;
-import org.apache.log4j.Logger;
import org.junit.Assert;
+import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
import com.cloud.utils.testcase.Log4jEnabledTestCase;
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({Profiler.class})
public class TestProfiler extends Log4jEnabledTestCase {
- protected final static Logger s_logger = Logger.getLogger(TestProfiler.class);
- private static final long MILLIS_FACTOR = 1000l;
- private static final int MARGIN = 100;
- // Profiler uses System.nanoTime which is not reliable on the millisecond
- // A sleep of 1000 milliseconds CAN be measured as 999 milliseconds
- // Therefore make the second larger, by 10% of the margin
- private static final long ONE_SECOND = 1000l + (MARGIN / 10);
- private static final double EXPONENT = 3d;
+ private static final long SLEEP_TIME_NANO = 1000000000L;
+ private static Profiler pf;
+
+ @Before
+ public void setUp() {
+ pf = new Profiler();
+ PowerMockito.mockStatic(System.class);
+ PowerMockito.when(System.nanoTime()).thenReturn(0L, SLEEP_TIME_NANO);
--- End diff --
@alexandrelimassantana , You are absolutely right, i've been reading without glasses. nice work.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-211359351
@swill thanks, I was expecting for this one a long time
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1445#discussion_r56799125
--- Diff: utils/src/test/java/com/cloud/utils/TestProfiler.java ---
@@ -19,84 +19,79 @@
package com.cloud.utils;
-import org.apache.log4j.Logger;
import org.junit.Assert;
+import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
import com.cloud.utils.testcase.Log4jEnabledTestCase;
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({Profiler.class})
public class TestProfiler extends Log4jEnabledTestCase {
- protected final static Logger s_logger = Logger.getLogger(TestProfiler.class);
- private static final long MILLIS_FACTOR = 1000l;
- private static final int MARGIN = 100;
- // Profiler uses System.nanoTime which is not reliable on the millisecond
- // A sleep of 1000 milliseconds CAN be measured as 999 milliseconds
- // Therefore make the second larger, by 10% of the margin
- private static final long ONE_SECOND = 1000l + (MARGIN / 10);
- private static final double EXPONENT = 3d;
+ private static final long SLEEP_TIME_NANO = 1000000000L;
+ private static Profiler pf;
+
+ @Before
+ public void setUp() {
+ pf = new Profiler();
+ PowerMockito.mockStatic(System.class);
+ PowerMockito.when(System.nanoTime()).thenReturn(0L, SLEEP_TIME_NANO);
--- End diff --
mocking the system return of the time does not seem right to me. it will be returning the same in start() and in stop(), don't you agree?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209026629
Just trying to help get things merged :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by pdube <gi...@git.apache.org>.
Github user pdube commented on the pull request:
https://github.com/apache/cloudstack/pull/1445#issuecomment-209023846
@rafaelweingartner @swill The only thing changed was the unit tests, which I ran. And the only code change he did was to remove useless variables. @swill also gave his LGTM higher
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1445
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Fixed Profiler's unit tests bugs.
Posted by alexandrelimassantana <gi...@git.apache.org>.
Github user alexandrelimassantana commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1445#discussion_r56808020
--- Diff: utils/src/test/java/com/cloud/utils/TestProfiler.java ---
@@ -19,84 +19,79 @@
package com.cloud.utils;
-import org.apache.log4j.Logger;
import org.junit.Assert;
+import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
import com.cloud.utils.testcase.Log4jEnabledTestCase;
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({Profiler.class})
public class TestProfiler extends Log4jEnabledTestCase {
- protected final static Logger s_logger = Logger.getLogger(TestProfiler.class);
- private static final long MILLIS_FACTOR = 1000l;
- private static final int MARGIN = 100;
- // Profiler uses System.nanoTime which is not reliable on the millisecond
- // A sleep of 1000 milliseconds CAN be measured as 999 milliseconds
- // Therefore make the second larger, by 10% of the margin
- private static final long ONE_SECOND = 1000l + (MARGIN / 10);
- private static final double EXPONENT = 3d;
+ private static final long SLEEP_TIME_NANO = 1000000000L;
+ private static Profiler pf;
+
+ @Before
+ public void setUp() {
+ pf = new Profiler();
+ PowerMockito.mockStatic(System.class);
+ PowerMockito.when(System.nanoTime()).thenReturn(0L, SLEEP_TIME_NANO);
--- End diff --
The statement on line 43 is saying that on the first call to nanoTime it will return 0 and the second call will return 10^9 ( 1 second in nanoseconds ).
when start() calls nanoTime() first, it gets 0 and when stop() calls it again, the result will be 10^9. That is how it simulates the passage of 1s without actually making the thread sleep for 1 second.
The tests previously written about this class are not intended to test the capability of the System class to capture time nor the Thread.sleep() method ( which was causing the test to fail as I stated because of the low priority the JVM's proccess have on different OS ). As it is right now, it stills test the same functionalities... Just without the sleep and System direct calls.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---