diff options
author | Martin Kirst | 2011-12-21 13:59:16 -0500 |
---|---|---|
committer | Paul Webster | 2011-12-21 14:16:30 -0500 |
commit | 827da075b5564198c2399b94eb0bce7d1375e3e9 (patch) | |
tree | 461a3ab2de8f03b346727de84383ef033cf74ff0 | |
parent | 66f6b21a942c5178fe68faa948ade7596f535b56 (diff) | |
download | eclipse.platform.ui-827da075b5564198c2399b94eb0bce7d1375e3e9.zip eclipse.platform.ui-827da075b5564198c2399b94eb0bce7d1375e3e9.tar.gz eclipse.platform.ui-827da075b5564198c2399b94eb0bce7d1375e3e9.tar.xz |
Bug 361121 - [Progress]DetailedProgressViewer's comparator violatesv20111221-1916
its general contract
Enhance the JobInfo compareTo
4 files changed, 383 insertions, 7 deletions
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/JobInfo.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/JobInfo.java index 5a59787..4430152 100644 --- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/JobInfo.java +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/JobInfo.java @@ -8,6 +8,7 @@ * Contributors: * IBM Corporation - initial API and implementation * Brock Janiczak <brockj@tpg.com.au> - Fix for Bug 123169 [Progress] NPE from JobInfo + * Martin W. Kirst <martin.kirst@s1998.tu-chemnitz.de> - jUnit test for Bug 361121 [Progress] DetailedProgressViewer's comparator violates its general contract *******************************************************************************/ package org.eclipse.ui.internal.progress; @@ -53,7 +54,7 @@ public class JobInfo extends JobTreeElement { * * @param enclosingJob */ - JobInfo(Job enclosingJob) { + protected JobInfo(Job enclosingJob) { this.job = enclosingJob; } @@ -154,10 +155,12 @@ public class JobInfo extends JobTreeElement { } } + // If equal prio, order by names if (job.getPriority() == job2.getPriority()) { return job.getName().compareTo(job2.getName()); } + // order by priority if (job.getPriority() > job2.getPriority()) { return -1; } @@ -181,15 +184,16 @@ public class JobInfo extends JobTreeElement { return 1; } - if (element.getJob().getState() == getJob().getState()) { - return compareJobs(element); - } + int thisState = getJob().getState(); + int anotherState = element.getJob().getState(); - if (getJob().getState() == Job.RUNNING) { - return -1; + // if equal job state, compare other job attributes + if (thisState == anotherState) { + return compareJobs(element); } - return 1; + // ordering by job states, Job.RUNNING should be ordered first + return (thisState > anotherState ? -1 : (thisState == anotherState ? 0 : 1)); } /** diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/JobInfoTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/JobInfoTest.java new file mode 100644 index 0000000..04e6fc1 --- /dev/null +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/JobInfoTest.java @@ -0,0 +1,235 @@ +/******************************************************************************* + * Copyright (c) 2011 IBM Corporation and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * IBM Corporation - initial API and implementation + * Martin W. Kirst <martin.kirst@s1998.tu-chemnitz.de> - jUnit test for Bug 361121 [Progress] DetailedProgressViewer's comparator violates its general contract + ******************************************************************************/ + +package org.eclipse.ui.tests.progress; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.List; + +import junit.framework.TestCase; + +import org.eclipse.core.internal.jobs.InternalJob; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.jobs.Job; +import org.eclipse.ui.internal.progress.JobInfo; + +public class JobInfoTest extends TestCase { + + + /** + * @see org.eclipse.core.internal.jobs.InternalJob + */ + static final int ABOUT_TO_RUN = 0x10; + /** + * @see org.eclipse.core.internal.jobs.InternalJob + */ + static final int ABOUT_TO_SCHEDULE = 0x20; + /** + * @see org.eclipse.core.internal.jobs.InternalJob + */ + static final int BLOCKED = 0x08; + /** + * @see org.eclipse.core.internal.jobs.InternalJob + */ + static final int YIELDING = 0x40; + + private List jobinfos = new ArrayList(); + + /** + * @throws java.lang.Exception + */ + protected void setUp() throws Exception { + int counter = 0; + counter = createAndAddJobInfos(false, false, ABOUT_TO_RUN, counter); + counter = createAndAddJobInfos(false, true, ABOUT_TO_RUN, counter); + counter = createAndAddJobInfos(true, false, ABOUT_TO_RUN, counter); + counter = createAndAddJobInfos(true, true, ABOUT_TO_RUN, counter); + + counter = createAndAddJobInfos(false, false, ABOUT_TO_SCHEDULE, counter); + counter = createAndAddJobInfos(false, true, ABOUT_TO_SCHEDULE, counter); + counter = createAndAddJobInfos(true, false, ABOUT_TO_SCHEDULE, counter); + counter = createAndAddJobInfos(true, true, ABOUT_TO_SCHEDULE, counter); + + counter = createAndAddJobInfos(false, false, Job.SLEEPING, counter); + counter = createAndAddJobInfos(false, true, Job.SLEEPING, counter); + counter = createAndAddJobInfos(true, false, Job.SLEEPING, counter); + counter = createAndAddJobInfos(true, true, Job.SLEEPING, counter); + + counter = createAndAddJobInfos(false, false, Job.WAITING, counter); + counter = createAndAddJobInfos(false, true, Job.WAITING, counter); + counter = createAndAddJobInfos(true, false, Job.WAITING, counter); + counter = createAndAddJobInfos(true, true, Job.WAITING, counter); + + counter = createAndAddJobInfos(false, false, Job.RUNNING, counter); + counter = createAndAddJobInfos(false, true, Job.RUNNING, counter); + counter = createAndAddJobInfos(true, false, Job.RUNNING, counter); + counter = createAndAddJobInfos(true, true, Job.RUNNING, counter); + + } + + /** + * Test that {@link org.eclipse.ui.internal.progress.JobInfo#compareTo(Object)} + * is valid implemented and complies to the interface method contract. + */ + public void testCompareToContractCompliance() { + for(int xi = 0; xi<this.jobinfos.size(); xi++) { + JobInfo x = (JobInfo) jobinfos.get(xi); + + for(int yi = 0; yi<this.jobinfos.size(); yi++) { + JobInfo y = (JobInfo) jobinfos.get(yi); + int xyResult = x.compareTo(y); + int yxResult = y.compareTo(x); + // sgn(compare(x, y)) == -sgn(compare(y, x)) for all x and y. + assertEquals(String.format("sgn(compare(%1$s, %2$s)) != -sgn(compare(%2$s, %1$s))", new Object[] { x, y}), + Math.round(Math.signum(xyResult)) , Math.round(-Math.signum(yxResult))); + + for(int zi = 0; zi<this.jobinfos.size(); zi++) { + JobInfo z = (JobInfo) jobinfos.get(zi); + int xzResult = x.compareTo(z); + int yzResult = y.compareTo(z); + // ((compare(x, y)>0) && (compare(y, z)>0)) implies compare(x, z)>0. + if(xyResult > 0) { + if(yzResult > 0) { + assertTrue(String.format("((compare(%1$s, %2$s)>0) && (compare(%2$s, %3$s)>0)) but not compare(%1$s, %3$s)>0", new Object[] {x, y, z}), + xzResult > 0); + } + } + else if(xyResult == 0) { + // compare(x, y)==0 implies that sgn(compare(x, z))==sgn(compare(y, z)) for all z. + assertEquals(String.format("compare(%1$s, %2$s)==0 but not that sgn(compare(%1$s, %3$s))==sgn(compare(%2$s, %3$s))", new Object[]{ x, y, z}), + Math.round(Math.signum(xzResult)) , Math.round(Math.signum(yzResult))); + } + } + + boolean consistentWithEquals = true; + // Optionally (compare(x, y)==0) == (x.equals(y)) + if(consistentWithEquals && xyResult == 0) { + assertTrue(String.format("compare(%1$s, %2$s)==0) == (%1$s.equals(%2$s)", new Object[] {x, y}), + x.equals(y)); + } + } + } + } + + /** + * @param user + * @param system + * @param jobstate + * @param counter + * @return + */ + private int createAndAddJobInfos(boolean user, boolean system, int jobstate, int counter) { + TestJob job; + JobInfo ji; + + job = new TestJob("Job" + (counter++)); + job.setUser(user); + job.setSystem(system); + job.setPriority(Job.INTERACTIVE); + job.setInternalJobState(jobstate); + ji = new ExtendedJobInfo(job); + jobinfos.add(ji); + + job = new TestJob("Job" + (counter++)); + job.setUser(user); + job.setSystem(system); + job.setPriority(Job.SHORT); + job.setInternalJobState(jobstate); + ji = new ExtendedJobInfo(job); + jobinfos.add(ji); + + job = new TestJob("Job" + (counter++)); + job.setUser(user); + job.setSystem(system); + job.setPriority(Job.LONG); + job.setInternalJobState(jobstate); + ji = new ExtendedJobInfo(job); + jobinfos.add(ji); + + job = new TestJob("Job" + (counter++)); + job.setUser(user); + job.setSystem(system); + job.setPriority(Job.BUILD); + job.setInternalJobState(jobstate); + ji = new ExtendedJobInfo(job); + jobinfos.add(ji); + + job = new TestJob("Job" + (counter++)); + job.setUser(user); + job.setSystem(system); + job.setPriority(Job.DECORATE); + job.setInternalJobState(jobstate); + ji = new ExtendedJobInfo(job); + jobinfos.add(ji); + + job = new TestJob("Job" + (counter++)); + job.setUser(user); + job.setSystem(system); + job.setPriority(Job.LONG); + job.setInternalJobState(jobstate); + ji = new ExtendedJobInfo(job); + jobinfos.add(ji); + + return counter; + } + + /* + * ======================================================================== + */ + + /** + * Only provides better readable {@link #toString()} method. + */ + private static class ExtendedJobInfo extends JobInfo { + + public ExtendedJobInfo(Job enclosingJob) { + super(enclosingJob); + } + + public String toString() { + return "ExtendedJobInfo [getName()=" + getJob().getName() + ", getPriority()=" + + getJob().getPriority() + ", getState()=" + getJob().getState() + + ", isSystem()=" + getJob().isSystem() + ", isUser()=" + getJob().isUser() + + "]"; + } + + } + + /** + * Enables access to internal state, by using reflection + * Provides better readable {@link #toString()} method. + */ + private static class TestJob extends Job { + + public TestJob(String name) { + super(name); + } + + protected IStatus run(IProgressMonitor monitor) { + throw new UnsupportedOperationException("Not implemented, because of just a unit test"); + } + + public void setInternalJobState(int state) { + try { + final Field field = InternalJob.class.getDeclaredField("flags"); + field.setAccessible(true); // hack for testing + field.set(this, new Integer(state)); + } catch (Exception e) { + e.printStackTrace(); + throw new RuntimeException(e); + } + } + + } +} diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/JobInfoTestOrdering.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/JobInfoTestOrdering.java new file mode 100644 index 0000000..c56f1c7 --- /dev/null +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/JobInfoTestOrdering.java @@ -0,0 +1,135 @@ +/******************************************************************************* + * Copyright (c) 2011 IBM Corporation and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * IBM Corporation - initial API and implementation + * Martin W. Kirst <martin.kirst@s1998.tu-chemnitz.de> - jUnit test for Bug 361121 [Progress] DetailedProgressViewer's comparator violates its general contract + ******************************************************************************/ + +package org.eclipse.ui.tests.progress; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import junit.framework.TestCase; + +import org.eclipse.core.internal.jobs.InternalJob; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.jobs.Job; +import org.eclipse.ui.internal.progress.JobInfo; + +public class JobInfoTestOrdering extends TestCase { + + private List jobinfos = new ArrayList(); + + /** + * @throws java.lang.Exception + */ + protected void setUp() throws Exception { + jobinfos.clear(); + int counter = 0; + TestJob job; + JobInfo ji; + + job = new TestJob("Job" + (counter++)); + job.setUser(true); + job.setSystem(false); + job.setPriority(Job.INTERACTIVE); + job.setInternalJobState(Job.NONE); // JOB STATE + ji = new ExtendedJobInfo(job); + jobinfos.add(ji); + + job = new TestJob("Job" + (counter++)); + job.setUser(true); + job.setSystem(false); + job.setPriority(Job.INTERACTIVE); + job.setInternalJobState(Job.SLEEPING); // JOB STATE + ji = new ExtendedJobInfo(job); + jobinfos.add(ji); + + job = new TestJob("Job" + (counter++)); + job.setUser(true); + job.setSystem(false); + job.setPriority(Job.INTERACTIVE); + job.setInternalJobState(Job.WAITING); // JOB STATE + ji = new ExtendedJobInfo(job); + jobinfos.add(ji); + + job = new TestJob("Job" + (counter++)); + job.setUser(true); + job.setSystem(false); + job.setPriority(Job.INTERACTIVE); + job.setInternalJobState(Job.RUNNING); // JOB STATE + ji = new ExtendedJobInfo(job); + jobinfos.add(ji); + + } + + /** + * Test that checks when jobs sorted by their state, the running ones + * are ordered to first place + */ + public void testJobStateOrdering() { + Collections.sort(jobinfos); + assertEquals(Job.RUNNING, ((JobInfo)jobinfos.get(0)).getJob().getState()); + assertEquals(Job.WAITING, ((JobInfo)jobinfos.get(1)).getJob().getState()); + assertEquals(Job.SLEEPING, ((JobInfo)jobinfos.get(2)).getJob().getState()); + assertEquals(Job.NONE, ((JobInfo)jobinfos.get(3)).getJob().getState()); + } + + /* + * ======================================================================== + */ + + /** + * Only provides better readable {@link #toString()} method. + */ + private static class ExtendedJobInfo extends JobInfo { + + public ExtendedJobInfo(Job enclosingJob) { + super(enclosingJob); + } + + public String toString() { + return "ExtendedJobInfo [getName()=" + getJob().getName() + ", getPriority()=" + + getJob().getPriority() + ", getState()=" + getJob().getState() + + ", isSystem()=" + getJob().isSystem() + ", isUser()=" + getJob().isUser() + + "]"; + } + + } + + /** + * Enables access to internal state, by using reflection + * Provides better readable {@link #toString()} method. + */ + private static class TestJob extends Job { + + public TestJob(String name) { + super(name); + } + + protected IStatus run(IProgressMonitor monitor) { + throw new UnsupportedOperationException("Not implemented, because of just a unit test"); + } + + public void setInternalJobState(int state) { + try { + final Field field = InternalJob.class.getDeclaredField("flags"); + field.setAccessible(true); // hack for testing + field.set(this, new Integer(state)); + } catch (Exception e) { + e.printStackTrace(); + throw new RuntimeException(e); + } + } + + } +} diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/ProgressTestSuite.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/ProgressTestSuite.java index b5185aa..eee8388 100644 --- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/ProgressTestSuite.java +++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/progress/ProgressTestSuite.java @@ -33,5 +33,7 @@ public class ProgressTestSuite extends TestSuite { public ProgressTestSuite() { addTest(new TestSuite(ProgressContantsTest.class)); addTest(new TestSuite(ProgressViewTests.class)); + addTest(new TestSuite(JobInfoTest.class)); + addTest(new TestSuite(JobInfoTestOrdering.class)); } } |