You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2022/05/12 21:54:33 UTC
[GitHub] [netbeans] lbownik opened a new pull request, #4112: added tests to org.opeide.util.TastTest & upgraded to JUnit4
lbownik opened a new pull request, #4112:
URL: https://github.com/apache/netbeans/pull/4112
moved TaskTest to JUnit4
Added test new tests to cover uncovered functionality.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1043336866
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,184 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
Review Comment:
thanks. Looks good.
Infinite wait is somewhat covered by test and job timeouts - so things can't get stuck forever.
When squashing please make sure to only leave the interesting parts in the commit message to describe what the changes do, so that we have less noise in the log. No need to mention all the individual review motivated steps in the squashed commit msg.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lbownik commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1043325497
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,184 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
Review Comment:
I change currentTimeMillis() to nanoTime() and expanded wait time to 10 miliseconds in both tests.
10 miliseconds steel feel "immediately" from users perspective and the false negative rate should be really low.
now these two tests, though imperfect guard us from "catastrophic" failures (like infinite wait); alternatively we can skip testing these use cases entirely;
if You accept, I'll squash 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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1043143230
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,184 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
Review Comment:
I don't like this. First of all, `System.currentTimeMillis()` is not monotonic. It can run backwards in some situations, e.g. when CPU cores change. This also essentially is trying to measure a 1ms delta which isn't even possible resolution wise. However bad resolution can be even an advantage here and cause less failures. (timer accuracy on windows for example has been traditionally very bad, every 3d engine has its own workaround for that)
what is worse: you can literally call `currentTimeMillis`() twice in a row and get different values as this quick test demonstrates (-> expecting a 0 delta as the tests do is highly problematic):
```java
public static void main(String[] args) {
while (true) {
if (System.currentTimeMillis() != System.currentTimeMillis()) {
System.out.println("test failure: someone please restart the test job");
}
}
}
```
The timer can also skip multiple ms at once.
`nanoTime` can be monotonic if the platform/HW supports it and if its enabled - I don't really know how the cloud is set up here or how other OSes outside of linux are set up.
We already have many unreliable tests as you can see, the current run failed for example in several jobs. Even if it rarely fails it is a problem since someone has to play whack a mole and both (limited) CPU time and dev time is wasted.
So my intuition here is to rather not add more unreliable tests.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien merged pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
mbien merged PR #4112:
URL: https://github.com/apache/netbeans/pull/4112
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1042605806
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -147,7 +310,7 @@ public synchronized void run () {
}
}
}
-
+ @Test
/*
* see issue #130265
*/
Review Comment:
nitpick: move below comment
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1043143230
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,184 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
Review Comment:
I don't like this. First of all, `System.currentTimeMillis()` is not monotonic. It can run backwards in some situations, e.g. when CPU cores change. This also essentially is trying to measure a 1ms delta which isn't even possible resolution wise. However bad resolution can be even an advantage here and cause less failures.
You can literally call `currentTimeMillis`() twice in a row and get different values as this quick test demonstrates:
```java
public static void main(String[] args) {
while (true) {
if (System.currentTimeMillis() != System.currentTimeMillis()) {
System.out.println("test failure: someone please restart the test job");
}
}
}
```
`nanoTime` can be monotonic if the platform/HW supports it and if its enabled - I don't really know how the cloud is set up here or how other OSes outside of linux are set up.
We already have many unreliable tests as you can see, the current run failed for example in several jobs. Even if it rarely fails it is a problem since someone has to play whack a mole and both (limited) CPU time and dev time is wasted.
So my intuition here is to rather not add more unreliable tests.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#issuecomment-1342993757
all green thats a good sign. merging. @lbownik thanks for sticking around
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lbownik commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1042589866
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,183 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
}
- @Override
- protected void setUp() throws Exception {
- LOG = Logger.getLogger("org.openide.util.Task." + getName());
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedWithTimeoutReturnsImmediately(final Task task)
+ throws Exception {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished(0);
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished(long) took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
}
+ //--------------------------------------------------------------------------
+ @Test
+ public void emptyTask_isImmediatelyFinished_andNeverWaits()
+ throws Exception {
+
+ assertFinished(Task.EMPTY);
+ assertWaitFinishedReturnsImmediately(Task.EMPTY);
+ assertWaitFinishedWithTimeoutReturnsImmediately(Task.EMPTY);
+ assertEquals("task null", Task.EMPTY.toString());
+ assertEquals("null", Task.EMPTY.debug());
+
+ Task empty = new Task(null);
+ assertFinished(empty);
+ assertWaitFinishedReturnsImmediately(empty);
+ assertWaitFinishedWithTimeoutReturnsImmediately(empty);
+ assertEquals("task null", empty.toString());
+ assertEquals("null", empty.debug());
+ }
+
+ //--------------------------------------------------------------------------
+ @Test
+ public void runningEmptyTask_doesNothing() {
+
+ try {
+ Task.EMPTY.run();
Review Comment:
this test is stronger than that.
it makes sure that empty tasks are acttually runnable (can be run without throwing enything)
assertNotNull(Task.EMPTY) does not verify this
this tests checks whether things we don't want to happen (run method throwing an exception) actually don't happen.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1043143230
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,184 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
Review Comment:
I don't like this. First of all, `System.currentTimeMillis()` is not monotonic. It can run backwards in some situations, e.g. when CPU cores change. This also essentially is trying to measure a 1ms delta which isn't even possible resolution wise. However bad resolution can be even an advantage here and cause less failures. (timer accuracy on windows for example has been traditionally very bad, every 3d engine has its own workaround for that)
You can literally call `currentTimeMillis`() twice in a row and get different values as this quick test demonstrates (-> expecting a 0 delta as the tests do is highly problematic):
```java
public static void main(String[] args) {
while (true) {
if (System.currentTimeMillis() != System.currentTimeMillis()) {
System.out.println("test failure: someone please restart the test job");
}
}
}
```
`nanoTime` can be monotonic if the platform/HW supports it and if its enabled - I don't really know how the cloud is set up here or how other OSes outside of linux are set up.
We already have many unreliable tests as you can see, the current run failed for example in several jobs. Even if it rarely fails it is a problem since someone has to play whack a mole and both (limited) CPU time and dev time is wasted.
So my intuition here is to rather not add more unreliable tests.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lbownik commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1043377760
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,184 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
Review Comment:
squashed
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] sdedic commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
sdedic commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1042564056
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,183 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
}
- @Override
- protected void setUp() throws Exception {
- LOG = Logger.getLogger("org.openide.util.Task." + getName());
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedWithTimeoutReturnsImmediately(final Task task)
+ throws Exception {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished(0);
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished(long) took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
}
+ //--------------------------------------------------------------------------
+ @Test
+ public void emptyTask_isImmediatelyFinished_andNeverWaits()
+ throws Exception {
+
+ assertFinished(Task.EMPTY);
+ assertWaitFinishedReturnsImmediately(Task.EMPTY);
+ assertWaitFinishedWithTimeoutReturnsImmediately(Task.EMPTY);
+ assertEquals("task null", Task.EMPTY.toString());
+ assertEquals("null", Task.EMPTY.debug());
+
+ Task empty = new Task(null);
+ assertFinished(empty);
+ assertWaitFinishedReturnsImmediately(empty);
+ assertWaitFinishedWithTimeoutReturnsImmediately(empty);
+ assertEquals("task null", empty.toString());
+ assertEquals("null", empty.debug());
+ }
+
+ //--------------------------------------------------------------------------
+ @Test
+ public void runningEmptyTask_doesNothing() {
+
+ try {
+ Task.EMPTY.run();
Review Comment:
this is not necessary, simple `assertNotNull(Task.EMPTY)`. Maybe the 1st test that checks that `public static final` object is not null ...
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,183 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
}
- @Override
- protected void setUp() throws Exception {
- LOG = Logger.getLogger("org.openide.util.Task." + getName());
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedWithTimeoutReturnsImmediately(final Task task)
+ throws Exception {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished(0);
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished(long) took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
}
+ //--------------------------------------------------------------------------
+ @Test
+ public void emptyTask_isImmediatelyFinished_andNeverWaits()
+ throws Exception {
+
+ assertFinished(Task.EMPTY);
+ assertWaitFinishedReturnsImmediately(Task.EMPTY);
+ assertWaitFinishedWithTimeoutReturnsImmediately(Task.EMPTY);
+ assertEquals("task null", Task.EMPTY.toString());
+ assertEquals("null", Task.EMPTY.debug());
+
+ Task empty = new Task(null);
+ assertFinished(empty);
+ assertWaitFinishedReturnsImmediately(empty);
+ assertWaitFinishedWithTimeoutReturnsImmediately(empty);
+ assertEquals("task null", empty.toString());
+ assertEquals("null", empty.debug());
+ }
+
+ //--------------------------------------------------------------------------
+ @Test
+ public void runningEmptyTask_doesNothing() {
+
+ try {
+ Task.EMPTY.run();
+ } catch (final NullPointerException e) {
+ fail("NullPointerException shall never happen.");
+ }
+ }
+
+ //--------------------------------------------------------------------------
+ @Test
+ public void runningTask_executesRunnableAndListeners() {
+
+ this.runHasBeenExecuted = false;
+ this.executedListenerTask = null;
+
+ Task task = new Task(() -> {
+ this.runHasBeenExecuted = true;
+ });
+ task.addTaskListener((t) -> {
+ this.executedListenerTask = t;
+ });
+
+ assertFalse(task.isFinished());
+ assertNotEquals("null", task.debug());
+
+ task.run();
+
+ assertTrue(this.runHasBeenExecuted);
+ assertEquals(task, this.executedListenerTask);
Review Comment:
I'd put even `assertSame` here.
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,183 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
}
- @Override
- protected void setUp() throws Exception {
- LOG = Logger.getLogger("org.openide.util.Task." + getName());
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedWithTimeoutReturnsImmediately(final Task task)
+ throws Exception {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished(0);
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished(long) took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
}
+ //--------------------------------------------------------------------------
+ @Test
+ public void emptyTask_isImmediatelyFinished_andNeverWaits()
+ throws Exception {
+
+ assertFinished(Task.EMPTY);
+ assertWaitFinishedReturnsImmediately(Task.EMPTY);
+ assertWaitFinishedWithTimeoutReturnsImmediately(Task.EMPTY);
+ assertEquals("task null", Task.EMPTY.toString());
+ assertEquals("null", Task.EMPTY.debug());
+
+ Task empty = new Task(null);
+ assertFinished(empty);
+ assertWaitFinishedReturnsImmediately(empty);
+ assertWaitFinishedWithTimeoutReturnsImmediately(empty);
+ assertEquals("task null", empty.toString());
+ assertEquals("null", empty.debug());
+ }
+
+ //--------------------------------------------------------------------------
+ @Test
+ public void runningEmptyTask_doesNothing() {
+
+ try {
+ Task.EMPTY.run();
+ } catch (final NullPointerException e) {
+ fail("NullPointerException shall never happen.");
+ }
+ }
+
+ //--------------------------------------------------------------------------
+ @Test
+ public void runningTask_executesRunnableAndListeners() {
+
+ this.runHasBeenExecuted = false;
+ this.executedListenerTask = null;
+
+ Task task = new Task(() -> {
+ this.runHasBeenExecuted = true;
+ });
+ task.addTaskListener((t) -> {
+ this.executedListenerTask = t;
+ });
+
+ assertFalse(task.isFinished());
+ assertNotEquals("null", task.debug());
+
+ task.run();
+
+ assertTrue(this.runHasBeenExecuted);
+ assertEquals(task, this.executedListenerTask);
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ @Test
+ public void runningTask_doesNotRunRemovedListeners() {
+
+ this.executedListenerTask = null;
+ TaskListener listener = (t) -> {
+ this.executedListenerTask = t;
+ };
+
+ Task task = new Task(() -> {
+ });
+ task.addTaskListener(listener);
+ task.removeTaskListener(listener);
+
+ task.run();
+
+ assertTrue(task.isFinished());
+ assertNull(this.executedListenerTask);
+ }
+
+ //--------------------------------------------------------------------------
+ @Test
+ public void finishedTask_executesAddedListenerImmediately() {
+
+ this.executedListenerTask = null;
+
+ Task task = new Task(() -> {
+ });
+ task.run();
+
+ assertNull(this.executedListenerTask);
+ assertTrue(task.isFinished());
+
+ task.addTaskListener((t) -> {
+ this.executedListenerTask = t;
+ });
+
+ assertEquals(task, this.executedListenerTask);
Review Comment:
this is an important feature, thanks for the test.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lbownik commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1043347476
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,184 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
Review Comment:
ok, I also spotted a n error in nono to milisecond ratio fized that in https://github.com/apache/netbeans/pull/4112/commits/0c00a1e99c3168a876b73279ce33326ace718c32
now squashing
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lbownik commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1043325497
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,184 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
Review Comment:
I changed currentTimeMillis() to nanoTime() in https://github.com/apache/netbeans/pull/4112/commits/31c8e4c8119ede109d0b37499d39684ebda61458 and expanded wait time to 10 miliseconds in both tests.
10 miliseconds steel feel "immediately" from users perspective and the false negative rate should be really low.
now these two tests, though imperfect guard us from "catastrophic" failures (like infinite wait); alternatively we can skip testing these use cases entirely;
if You accept, I'll squash 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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#issuecomment-1341359442
added the platform test jobs and restarted the worklow
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lbownik commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1042614362
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -147,7 +310,7 @@ public synchronized void run () {
}
}
}
-
+ @Test
/*
* see issue #130265
*/
Review Comment:
done in commit https://github.com/apache/netbeans/pull/4112/commits/3d0f3c0bd2831bcb76cb6001d4c5e25a17b8917d
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1043143230
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,184 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
Review Comment:
I don't like this. First of all, `System.currentTimeMillis()` is not monotonic. It can run backwards in some situations, e.g. when CPU cores change. This also essentially is trying to measure a 1ms delta which isn't even possible resolution wise. However bad resolution can be even an advantage here and cause less failures. (timer accuracy on windows for example has been traditionally very bad, every 3d engine has its own workaround for that)
You can literally call `currentTimeMillis`() twice in a row and get different values as this quick test demonstrates:
```java
public static void main(String[] args) {
while (true) {
if (System.currentTimeMillis() != System.currentTimeMillis()) {
System.out.println("test failure: someone please restart the test job");
}
}
}
```
`nanoTime` can be monotonic if the platform/HW supports it and if its enabled - I don't really know how the cloud is set up here or how other OSes outside of linux are set up.
We already have many unreliable tests as you can see, the current run failed for example in several jobs. Even if it rarely fails it is a problem since someone has to play whack a mole and both (limited) CPU time and dev time is wasted.
So my intuition here is to rather not add more unreliable tests.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] mbien commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1043198002
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,184 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
Review Comment:
at the very least:
* use `nanoTime`
* allow a delta of 1-2 ms
Most tests should have enough heap to hopefully not trigger GC in situations like this (which would cause failure too).
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lbownik commented on a diff in pull request #4112: added tests to org.openide.util.TaskTest & upgraded to JUnit4
Posted by GitBox <gi...@apache.org>.
lbownik commented on code in PR #4112:
URL: https://github.com/apache/netbeans/pull/4112#discussion_r1042597605
##########
platform/openide.util/test/unit/src/org/openide/util/TaskTest.java:
##########
@@ -21,25 +21,183 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Test;
import org.netbeans.junit.Log;
-import org.netbeans.junit.NbTestCase;
-import org.openide.util.Exceptions;
-import org.openide.util.Task;
+import static java.lang.System.currentTimeMillis;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
+import org.junit.Ignore;
-public class TaskTest extends NbTestCase {
- private Logger LOG;
- public TaskTest(String testName) {
- super(testName);
+public class TaskTest {
+ private static final Logger LOG = Logger.getLogger("org.openide.util.TaskTest");
+
+ private volatile boolean runHasBeenExecuted = false;
+ private volatile Task executedListenerTask = null;
+
+ //--------------------------------------------------------------------------
+ private static void assertFinished(final Task task) {
+
+ assertTrue(task.isFinished());
+ }
+
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedReturnsImmediately(final Task task) {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished();
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished() took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
}
- @Override
- protected void setUp() throws Exception {
- LOG = Logger.getLogger("org.openide.util.Task." + getName());
+ //--------------------------------------------------------------------------
+ private static void assertWaitFinishedWithTimeoutReturnsImmediately(final Task task)
+ throws Exception {
+
+ final long begin = currentTimeMillis();
+ task.waitFinished(0);
+ final long duration = currentTimeMillis() - begin;
+ // shorter than 1 milisecond
+ assertEquals("The Task.waitFinished(long) took longer than milisecond. "
+ + "This is not neseserily a bug.", 0, duration);
}
+ //--------------------------------------------------------------------------
+ @Test
+ public void emptyTask_isImmediatelyFinished_andNeverWaits()
+ throws Exception {
+
+ assertFinished(Task.EMPTY);
+ assertWaitFinishedReturnsImmediately(Task.EMPTY);
+ assertWaitFinishedWithTimeoutReturnsImmediately(Task.EMPTY);
+ assertEquals("task null", Task.EMPTY.toString());
+ assertEquals("null", Task.EMPTY.debug());
+
+ Task empty = new Task(null);
+ assertFinished(empty);
+ assertWaitFinishedReturnsImmediately(empty);
+ assertWaitFinishedWithTimeoutReturnsImmediately(empty);
+ assertEquals("task null", empty.toString());
+ assertEquals("null", empty.debug());
+ }
+
+ //--------------------------------------------------------------------------
+ @Test
+ public void runningEmptyTask_doesNothing() {
+
+ try {
+ Task.EMPTY.run();
+ } catch (final NullPointerException e) {
+ fail("NullPointerException shall never happen.");
+ }
+ }
+
+ //--------------------------------------------------------------------------
+ @Test
+ public void runningTask_executesRunnableAndListeners() {
+
+ this.runHasBeenExecuted = false;
+ this.executedListenerTask = null;
+
+ Task task = new Task(() -> {
+ this.runHasBeenExecuted = true;
+ });
+ task.addTaskListener((t) -> {
+ this.executedListenerTask = t;
+ });
+
+ assertFalse(task.isFinished());
+ assertNotEquals("null", task.debug());
+
+ task.run();
+
+ assertTrue(this.runHasBeenExecuted);
+ assertEquals(task, this.executedListenerTask);
Review Comment:
fixed in commit https://github.com/apache/netbeans/pull/4112/commits/8802f0171d7fff64f99ffb63a5f1ff748f613917
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists