Simplify and improve several assertions in org.eclipse.ui.tests

- Replace fail statements with proper assertions
- Remove unused code after fail statements
- Remove obsolete error messages

Author: HeikoKlare

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/internal/ide/DirectoryProposalContentAssistTestCase.java

@@ -15,7 +15,6 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 import java.util.Arrays;
 import java.util.Objects;
@@ -87,9 +86,7 @@ public void assertProposalSize(int size) {
 		Shell[] shells = getFieldAssistWindow().getDisplay().getShells();
 		Optional<Table> tableOptional = Arrays.stream(shells).map(this::retrieveTable)
 				.filter(Objects::nonNull).findFirst();
-		if (!tableOptional.isPresent()) {
-			fail("Couldn't assert pop-up proposal size - pop-up seems closed.");
-		}
+		assertTrue("Couldn't assert pop-up proposal size - pop-up seems closed.", tableOptional.isPresent());
 		TableItem[] proposals = tableOptional.get().getItems();
 
 		assertEquals("Proposal size must be " + size, size, proposals.length);

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/activities/PersistanceTest.java

@@ -15,9 +15,9 @@
 package org.eclipse.ui.tests.activities;
 
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.activities.IActivity;
@@ -44,9 +44,7 @@ public void testCategoryPermutations() throws NotDefinedException {
 		assertNotNull(category.getDescription());
 
 		for (String string : manager.getDefinedCategoryIds()) {
-			if (manager.getCategory(string).getName().equals("org.eclipse.ui.PT.C3")) {
-				fail("Found category that should not be.");
-			}
+			assertNotEquals("org.eclipse.ui.PT.C3", manager.getCategory(string).getName());
 		}
 	}
 
@@ -82,9 +80,7 @@ public void testActivityPermutations() throws NotDefinedException {
 		assertNotNull(activity.getDescription());
 
 		for (String string : manager.getDefinedActivityIds()) {
-			if (manager.getActivity(string).getName().equals("org.eclipse.ui.PT.A3")) {
-				fail("Found activity that should not be.");
-			}
+			assertNotEquals("org.eclipse.ui.PT.A3", manager.getActivity(string).getName());
 		}
 	}
 }

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/IAggregateWorkingSetTest.java

@@ -307,23 +307,8 @@ public void XXXtestWorkingSetSaveRestoreAggregates() throws Throwable {
 			assertNotNull("Unable to save/restore correctly", restoredC);
 			assertNotNull("Unable to save/restore correctly", restoredB);
 
-			IWorkingSet[] componenents1 = wSetB.getComponents();
-			IWorkingSet[] componenents2 = restoredB.getComponents();
-
-			if (componenents1.length != componenents2.length) {
-				assertArrayEquals(nameB + " has lost data in the process of save/restore: " + restoredB,
-						wSetB.getComponents(), restoredB.getComponents());
-			} else {
-				for (int i = 0; i < componenents1.length; i++) {
-					if (!componenents1[i].equals(componenents2[i])) {
-						assertEquals(nameB + " has lost data in the process of save/restore: " + restoredB,
-								componenents1[i].toString(), componenents2[i].toString());
-						fail("equals() and toString() do not match for: " + componenents1[i] + " and "
-								+ componenents2[i]);
-					}
-				}
-			}
-
+			assertArrayEquals(nameB + " has lost data in the process of save/restore: " + restoredB,
+					wSetB.getComponents(), restoredB.getComponents());
 		} finally {
 			// restore
 			IWorkingSet set = manager.getWorkingSet(nameA);

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/IEditorPartTest.java

@@ -137,9 +137,7 @@ public void testGetShellAfterClose() throws Throwable {
 		}
 
 		List<IStatus> list = errors.get("org.eclipse.ui.workbench");
-		if (list == null || list.isEmpty()) {
-			fail("No error reported on accessing shell after part disposal");
-		}
+		assertFalse("No error reported on accessing shell after part disposal", list == null || list.isEmpty());
 		assertEquals(1, list.size());
 		Throwable ex = list.get(0).getException();
 		assertTrue("Unexpected exception: " + ex, ex instanceof IllegalStateException);

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/ModalContextCrashTest.java

@@ -14,6 +14,7 @@
  *******************************************************************************/
 package org.eclipse.ui.tests.concurrency;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.fail;
 
@@ -39,6 +40,7 @@ public void testCrash() throws Exception {
 		if (Thread.interrupted()) {
 			fail("Thread was interrupted at end of test");
 		}
+		assertFalse(Thread.interrupted());
 	}
 
 	private static final class CrashingRunnable implements IRunnableWithProgress, IThreadListener {

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/NestedSyncExecDeadlockTest.java

@@ -14,7 +14,7 @@
  **********************************************************************/
 package org.eclipse.ui.tests.concurrency;
 
-import static org.junit.Assert.fail;
+import static org.junit.Assert.assertFalse;
 
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
@@ -111,16 +111,12 @@ public void tearDown() throws Exception {
 	@Test
 	public void testDeadlock() throws Exception {
 		doTest(1000 * 30); // 30 secs almost always locks
-		if (Thread.interrupted()) {
-			fail("Thread was interrupted at end of test");
-		}
+		assertFalse(Thread.interrupted());
 	}
 
 	@Test
 	public void testOK() throws Exception {
 		doTest(0); // 0 rarely locks
-		if (Thread.interrupted()) {
-			fail("Thread was interrupted at end of test");
-		}
+		assertFalse(Thread.interrupted());
 	}
 }

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/SyncExecWhileUIThreadWaitsForLock.java

@@ -15,6 +15,7 @@
 package org.eclipse.ui.tests.concurrency;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -86,9 +87,7 @@ public void tearDown() throws Exception {
 
 	@Test
 	public void testDeadlock() throws Exception {
-		if (Thread.interrupted()) {
-			fail("Thread was interrupted at start of test");
-		}
+		assertFalse(Thread.interrupted());
 		final ILock lock = Job.getJobManager().newLock();
 		final boolean[] blocked = new boolean[] {false};
 		final boolean[] lockAcquired= new boolean[] {false};
@@ -163,9 +162,6 @@ public void run() {
 		assertTrue("Unexpected: " + label, label.startsWith("UI thread"));
 		label = ((LogEntry) children[1]).getMessage();
 		assertTrue("Unexpected: " + label, label.startsWith("SyncExec"));
-		if (Thread.interrupted()) {
-			// TODO: re-enable this check after bug 505920 is fixed
-			// fail("Thread was interrupted at end of test");
-		}
+		assertFalse(Thread.interrupted());
 	}
 }

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/TestBug105491.java

@@ -13,6 +13,7 @@
  *******************************************************************************/
 package org.eclipse.ui.tests.concurrency;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.fail;
 
 import java.lang.reflect.InvocationTargetException;
@@ -79,9 +80,7 @@ public void threadChange(Thread thread) {
 	 */
 	@Test
 	public void testBug() throws CoreException {
-		if (Thread.interrupted()) {
-			fail("Thread was interrupted at start of test");
-		}
+		assertFalse(Thread.interrupted());
 
 		if (Util.isWindows()) {
 			// unstable on Windows with 2 cores, see bug 543693
@@ -98,8 +97,6 @@ public void testBug() throws CoreException {
 				// ignore
 			}
 		}, workspace.getRoot(), IResource.NONE, null);
-		if (Thread.interrupted()) {
-			fail("Thread was interrupted at end of test");
-		}
+		assertFalse(Thread.interrupted());
 	}
 }

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/concurrency/TransferRuleTest.java

@@ -13,10 +13,6 @@
  *******************************************************************************/
 package org.eclipse.ui.tests.concurrency;
 
-import static org.junit.Assert.fail;
-
-import java.lang.reflect.InvocationTargetException;
-
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.jobs.ISchedulingRule;
 import org.eclipse.core.runtime.jobs.Job;
@@ -87,7 +83,7 @@ public void threadChange(Thread thread) {
 	}
 
 	@Test
-	public void testModalContextTransfer() throws InvocationTargetException, InterruptedException {
+	public void testModalContextTransfer() throws Throwable {
 		ISchedulingRule rule = new TestRule();
 		TestRunnable runnable = new TestRunnable(rule);
 		try {
@@ -101,8 +97,7 @@ public void testModalContextTransfer() throws InvocationTargetException, Interru
 		}
 		//check the runnable for errors
 		if (runnable.error != null) {
-			runnable.error.printStackTrace();
-			fail("1.2");
+			throw runnable.error;
 		}
 	}
 }

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/datatransfer/ExportArchiveFileOperationTest.java

@@ -19,6 +19,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Enumeration;
 import java.util.HashSet;
 import java.util.List;
@@ -327,9 +328,7 @@ protected void doSetUp() throws Exception {
 			new File(FileSystemHelper.getRandomLocation(FileSystemHelper.getTempDir())
 				.toOSString());
 		localDirectory = destination.getAbsolutePath();
-		if (!destination.mkdirs()) {
-			fail("Could not set up destination directory for " + getName());
-		}
+		assertTrue(destination.mkdirs());
 		setUpData();
 		flattenPaths = false;
 		excludeProjectPath = false;
@@ -344,9 +343,7 @@ protected void doTearDown() throws Exception {
 			File[] files = root.listFiles();
 			if (files != null){
 				for (File file : files) {
-					if (!file.delete()) {
-						fail("Could not delete " + file.getAbsolutePath());
-					}
+					assertTrue("Could not delete " + file.getAbsolutePath(), file.delete());
 				}
 			}
 			root.delete();
@@ -471,16 +468,13 @@ private void verifyFiles(List<String> files) {
 	}
 
 	private void verifyFile(String entryName){
-		for (String fileName : fileNames) {
-			boolean dotProjectFileShouldBePresent = ".project".equals(entryName) && !flattenPaths && !excludeProjectPath;
-			if (fileName.equals(entryName) || dotProjectFileShouldBePresent) {
-				return;
-			}
-		}
 		if (entryName.equals("org.eclipse.core.resources.prefs")) {
 			return;
 		}
-		fail("Could not find file named: " + entryName);
+		assertTrue("Could not find file named: " + entryName, Arrays.stream(fileNames).anyMatch(fileName -> {
+			boolean dotProjectFileShouldBePresent = ".project".equals(entryName) && !flattenPaths && !excludeProjectPath;
+			return fileName.equals(entryName) || dotProjectFileShouldBePresent;
+		}));
 	}
 
 	private void verifyFolders(Set<String> folderNames) {
@@ -489,11 +483,8 @@ private void verifyFolders(Set<String> folderNames) {
 				continue;
 			}
 			if (!isDirectory(folderName)){
-				if (flattenPaths) {
-					fail(folderName + " is not an expected folder");
-				} else if (!project.getName().equals(folderName)) {
-					fail(folderName + " is not an expected folder");
-				}
+				assertFalse(folderName + " is not an expected folder",
+						flattenPaths || !project.getName().equals(folderName));
 			}
 		}
 	}