[ 
https://issues.apache.org/jira/browse/PROTON-1958?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16651822#comment-16651822
 ] 

ASF GitHub Bot commented on PROTON-1958:
----------------------------------------

Github user jdanekrh commented on the issue:

    https://github.com/apache/qpid-proton-j/pull/19
  
    Thanks. I took away the example change.
    
    About the test. What happens is that PriorityQueue dequeues equal items in 
arbitrary order. So the potential user-facing problem is that user creates two 
tasks with the intention of first running first, which would not happen due to 
the bug.
    
    My only idea for deterministic test would be something like
    
    ```
    public class TaskImplTest {
        @Test
        public void testCompareTo_greaterDeadlineObjectIsGreater() {
            TaskImpl t42 = new TaskImpl(42);
            TaskImpl t24 = new TaskImpl(24);
            Assert.assertTrue(t24.compareTo(t42) <= 0);
        }
    
        @Test
        public void testCompareTo_equalDeadlineLaterCreatedObjectIsGreater() {
            TaskImpl t42a = new TaskImpl(42);
            TaskImpl t42b = new TaskImpl(42);
            Assert.assertTrue(t42a.compareTo(t42b) <= 0);
        }
    }
    ```



> compareTo for tasks with same deadline is broken
> ------------------------------------------------
>
>                 Key: PROTON-1958
>                 URL: https://issues.apache.org/jira/browse/PROTON-1958
>             Project: Qpid Proton
>          Issue Type: Bug
>          Components: proton-j
>    Affects Versions: proton-j-0.29.0
>            Reporter: Jiri Daněk
>            Priority: Major
>
> Looking at the implementation, it seems that every instance of TaskImpl has 
> {{this.counter == 0}}. This seems wrong. The {{count}} field should be static 
> for the instance id to work. 
> {code}
> public class TaskImpl implements Task, Comparable<TaskImpl> {
> [...]
>     private final AtomicInteger count = new AtomicInteger();
> [...]
>     public TaskImpl(long deadline) {
>         this.deadline = deadline;
>         this.counter = count.getAndIncrement();
>     }
>     @Override
>     public int compareTo(TaskImpl other) {
>         int result;
>         if (deadline < other.deadline) {
>             result = -1;
>         } else if (deadline > other.deadline) {
>             result = 1;
>         } else {
>             result = counter - other.counter;
>         }
>         return result;
>     }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to