Move leaked shell check to CloseTestWindowsRule

The UITestCase contains a leaked shell check that would be better places
in a test rule and fits to the already existing CloseTestWindowsRule.

This change integrates the leaked shell check into the
CloseTestWindowsRule.

Author: HeikoKlare

tests/org.eclipse.ui.tests.harness/src/org/eclipse/ui/tests/harness/util/CloseTestWindowsRule.java

@@ -16,30 +16,41 @@
 package org.eclipse.ui.tests.harness.util;
 
 import static org.eclipse.ui.tests.harness.util.UITestUtil.processEvents;
+import static org.junit.Assert.assertEquals;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 
+import org.eclipse.swt.widgets.Shell;
 import org.eclipse.ui.IWindowListener;
 import org.eclipse.ui.IWorkbenchWindow;
 import org.eclipse.ui.PlatformUI;
 import org.junit.rules.ExternalResource;
 
 public class CloseTestWindowsRule extends ExternalResource {
 
-	private boolean enabled = true;
-
 	private final List<IWorkbenchWindow> testWindows;
 
 	private TestWindowListener windowListener;
 
+	private Set<Shell> preExistingShells;
+
+	private final boolean checkForLeaks;
+
 	public CloseTestWindowsRule() {
+		this(true);
+	}
+
+	public CloseTestWindowsRule(boolean checkForLeaks) {
 		testWindows = new ArrayList<>(3);
+		this.checkForLeaks = checkForLeaks;
 	}
 
 	@Override
 	protected void before() throws Exception {
 		addWindowListener();
+		this.preExistingShells = Set.of(PlatformUI.getWorkbench().getDisplay().getShells());
 	}
 
 	@Override
@@ -48,6 +59,19 @@ protected void after() {
 		processEvents();
 		closeAllTestWindows();
 		processEvents();
+		// Check for shell leak.
+		List<String> leakedModalShellTitles = new ArrayList<>();
+		Shell[] shells = PlatformUI.getWorkbench().getDisplay().getShells();
+		for (Shell shell : shells) {
+			if (!shell.isDisposed() && !preExistingShells.contains(shell)) {
+				leakedModalShellTitles.add(shell.getText());
+				shell.close();
+			}
+		}
+		if (checkForLeaks) {
+			assertEquals("Test leaked modal shell: [" + String.join(", ", leakedModalShellTitles) + "]", 0,
+				leakedModalShellTitles.size());
+		}
 	}
 
 	/**
@@ -78,10 +102,6 @@ private void closeAllTestWindows() {
 		testWindows.clear();
 	}
 
-	public void setEnabled(boolean enabled) {
-		this.enabled = enabled;
-	}
-
 	class TestWindowListener implements IWindowListener {
 		@Override
 		public void windowActivated(IWorkbenchWindow window) {
@@ -95,16 +115,12 @@ public void windowDeactivated(IWorkbenchWindow window) {
 
 		@Override
 		public void windowClosed(IWorkbenchWindow window) {
-			if (enabled) {
-				testWindows.remove(window);
-			}
+			testWindows.remove(window);
 		}
 
 		@Override
 		public void windowOpened(IWorkbenchWindow window) {
-			if (enabled) {
-				testWindows.add(window);
-			}
+			testWindows.add(window);
 		}
 	}
 }

tests/org.eclipse.ui.tests.harness/src/org/eclipse/ui/tests/harness/util/UITestCase.java

@@ -16,12 +16,6 @@
  *******************************************************************************/
 package org.eclipse.ui.tests.harness.util;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Set;
-
-import org.eclipse.swt.widgets.Shell;
-import org.eclipse.ui.PlatformUI;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -46,8 +40,6 @@ public abstract class UITestCase extends TestCase {
 	 */
 	private final CloseTestWindowsRule closeTestWindows = new CloseTestWindowsRule();
 
-	private Set<Shell> preExistingShells;
-
 	/**
 	 * Required to preserve the existing logging output when running tests with
 	 * {@link BlockJUnit4ClassRunner}.
@@ -97,11 +89,9 @@ protected void trace(String msg) {
 	public final void setUp() throws Exception {
 		super.setUp();
 		closeTestWindows.before();
-		this.preExistingShells = Set.of(PlatformUI.getWorkbench().getDisplay().getShells());
 		String name = runningTest != null ? runningTest : this.getName();
 		trace(TestRunLogUtil.formatTestStartMessage(name));
 		doSetUp();
-
 	}
 
 	/**
@@ -129,18 +119,7 @@ public final void tearDown() throws Exception {
 		String name = runningTest != null ? runningTest : this.getName();
 		trace(TestRunLogUtil.formatTestFinishedMessage(name));
 		doTearDown();
-
-		// Check for shell leak.
-		List<String> leakedModalShellTitles = new ArrayList<>();
-		Shell[] shells = PlatformUI.getWorkbench().getDisplay().getShells();
-		for (Shell shell : shells) {
-			if (!shell.isDisposed() && !preExistingShells.contains(shell)) {
-				leakedModalShellTitles.add(shell.getText());
-				shell.close();
-			}
-		}
-		assertEquals("Test leaked modal shell: [" + String.join(", ", leakedModalShellTitles) + "]", 0,
-				leakedModalShellTitles.size());
+		closeTestWindows.after();
 	}
 
 	/**
@@ -151,7 +130,6 @@ public final void tearDown() throws Exception {
 	 * Subclasses may extend.
 	 */
 	protected void doTearDown() throws Exception {
-		closeTestWindows.after();
 	}
 
 }

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/stress/OpenCloseTest.java

@@ -59,7 +59,7 @@ public class OpenCloseTest {
 	private IWorkbenchPage page;
 
 	@Rule
-	public CloseTestWindowsRule closeTestWindows = new CloseTestWindowsRule();
+	public CloseTestWindowsRule closeTestWindows = new CloseTestWindowsRule(false);
 
 	@Before
 	public void setup() {