Skip to content

Commit

Permalink
VirtualTableViewerTest: improve table update + force active shell ecl…
Browse files Browse the repository at this point in the history 
…ipse-platform#1005

Several test cases of VirtualTableViewerTest randomly fail. This
includes explicit failures of testRenameWithSorter() and testContains()
and silent failures of other test cases that simply return successfully
even if the test actually failed. The reason is a missing processing of
an update of the table data, which is most likely caused by the shell
not having focus.

With this change, the test execution ensures that in cases where an
update of the table data is expected, the shell is forced active and
events are processed until the data is finally set properly. In
addition, early returns producing silent failures are replaced by
assertions leading to explicit failures in case the condition is still
not met.

Contributes to
eclipse-platform#1583
May fix eclipse-platform#1005
  • Loading branch information
@HeikoKlare
HeikoKlare committed Apr 29, 2024
1 parent f44acf3 commit ba1643e
Showing 1 changed file with 23 additions and 63 deletions.
86 changes: 23 additions & 63 deletions ...s/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/VirtualTableViewerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;

import java.time.Duration;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
Expand All @@ -34,19 +35,14 @@
import org.eclipse.swt.widgets.TableItem;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TestName;

/**
* The TableViewerTest is a test of the SWT#VIRTUAL support in TableViewers,
*/
public class VirtualTableViewerTest extends TableViewerTest {

@Rule
public TestName testName = new TestName();

Set<TableItem> visibleItems = new HashSet<>();
private static final Duration TABLE_DATA_UPDATE_TIMEOUT = Duration.ofSeconds(5);

/**
* Checks if the virtual tree / table functionality can be tested in the current
Expand All @@ -57,13 +53,13 @@ public class VirtualTableViewerTest extends TableViewerTest {
* automated tests to fail. See
* https://bugs.eclipse.org/bugs/show_bug.cgi?id=118919 .
*/
protected boolean setDataCalled = false;
protected volatile boolean setDataCalled = false;

@Before
@Override
public void setUp() {
super.setUp();
processEvents(); // run events for SetData precondition test
waitForDataToBeSet();
}

@Override
Expand All @@ -73,32 +69,31 @@ protected int getShellStyle() {

@Override
protected TableViewer createTableViewer(Composite parent) {
visibleItems = new HashSet<>();
TableViewer viewer = new TableViewer(parent, SWT.VIRTUAL | SWT.MULTI);
viewer.setUseHashlookup(true);
final Table table = viewer.getTable();
table.addListener(SWT.SetData, event -> {
setDataCalled = true;
TableItem item = (TableItem) event.item;
visibleItems.add(item);
});
table.addListener(SWT.SetData, event -> setDataCalled = true);
return viewer;
}

/**
* Checks if update occurred. Updates for virtual items will be skipped if, for
* instance, another window is in the foreground.
*
* @return <code>true</code> if update occurred
* Updates the table, also forcing the containing shell to be active in order to
* process according events.
*/
protected boolean updateTable() {
private void updateTable() {
setDataCalled = false;
((TableViewer) fViewer).getControl().update();
if (setDataCalled) {
return true;
fViewer.refresh();
fViewer.getControl().update();
waitForDataToBeSet();
}

private void waitForDataToBeSet() {
Duration timeoutEnd = Duration.ofMillis(System.currentTimeMillis()).plus(TABLE_DATA_UPDATE_TIMEOUT);
while (!setDataCalled && !timeoutEnd.minusMillis(System.currentTimeMillis()).isNegative()) {
fShell.forceActive();
processEvents();
}
System.err.println("SWT.SetData is not received. Cancelled test " + testName.getMethodName());
return false;
assertTrue("waiting for setting table data timed out", setDataCalled);
}

/**
Expand All @@ -107,7 +102,7 @@ protected boolean updateTable() {
* @return TableItem[]
*/
private TableItem[] getVisibleItems() {
return visibleItems.toArray(new TableItem[visibleItems.size()]);
return ((TableViewer) fViewer).getTable().getItems();
}

@Test
Expand All @@ -130,44 +125,24 @@ protected int getItemCount() {
@Override
public void testFilter() {
ViewerFilter filter = new TestLabelFilter();
visibleItems = new HashSet<>();
fViewer.addFilter(filter);
if (!updateTable()) {
return;
}
assertEquals("filtered count", 5, getItemCount());

visibleItems = new HashSet<>();
fViewer.removeFilter(filter);
if (!updateTable()) {
return;
}
assertEquals("unfiltered count", 10, getItemCount());
}

@Test
@Override
public void testSetFilters() {
ViewerFilter filter = new TestLabelFilter();
visibleItems = new HashSet<>();
fViewer.setFilters(filter, new TestLabelFilter2());
if (!updateTable()) {
return;
}
assertEquals("2 filters count", 1, getItemCount());

visibleItems = new HashSet<>();
fViewer.setFilters(filter);
if (!updateTable()) {
return;
}
assertEquals("1 filtered count", 5, getItemCount());

visibleItems = new HashSet<>();
fViewer.setFilters();
if (!updateTable()) {
return;
}
assertEquals("unfiltered count", 10, getItemCount());
}

Expand Down Expand Up @@ -204,21 +179,13 @@ public void testInsertSiblingWithSorter() {
@Test
@Override
public void testRenameWithFilter() {
if (!setDataCalled) {
System.err.println("SWT.SetData is not received. Cancelled test " + testName.getMethodName());
return;
}
fViewer.addFilter(new TestLabelFilter());
if (!updateTable()) {
return;
}
TestElement first = fRootElement.getFirstChild();
first.setLabel("name-1111"); // should disappear
((TableViewer) fViewer).getControl().update();
assertNull("changed sibling is not visible", fViewer.testFindItem(first));
updateTable();
assertNull("changed sibling is still visible", fViewer.testFindItem(first));
first.setLabel("name-2222"); // should reappear
fViewer.refresh();
((TableViewer) fViewer).getControl().update();
updateTable();
assertNotNull("changed sibling is not visible", fViewer.testFindItem(first));
}

Expand All @@ -230,14 +197,10 @@ public void testSetInput() {
@Test
@Override
public void testRenameWithSorter() {
// Call update to make sure the viewer is in a correct state
// At least on MacOSX I get failures without this call
((TableViewer) fViewer).getControl().update();
fViewer.setComparator(new TestLabelComparator());
TestElement first = fRootElement.getFirstChild();
first.setLabel("name-9999");
String newElementLabel = first.toString();
((TableViewer) fViewer).getControl().update();
assertEquals("sorted first", newElementLabel, getItemText(0));
}

Expand All @@ -249,15 +212,12 @@ public void testSorter() {
String firstLabel = first.toString();
String lastLabel = last.toString();

((TableViewer) fViewer).getControl().update();
assertEquals("unsorted", firstLabel, getItemText(0));
fViewer.setComparator(new TestLabelComparator());

((TableViewer) fViewer).getControl().update();
assertEquals("reverse sorted", lastLabel, getItemText(0));

fViewer.setComparator(null);
((TableViewer) fViewer).getControl().update();
assertEquals("unsorted", firstLabel, getItemText(0));
}

Expand Down

0 comments on commit ba1643e

Please sign in to comment.