# HG changeset patch # User Anirudhan Mukundan # Date 1467296427 14400 # Node ID cbf98b5f488d99fcd22a7049e61b7067297a27ac # Parent df9c589c6e0c8a194a30c0eca6632b282c11416e HeapDumpDetailsController handles empty search string correctly PR3065 Reviewed-by: aazores, jerboaa Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-June/020016.html diff -r df9c589c6e0c -r cbf98b5f488d common/test/src/main/java/com/redhat/thermostat/testutils/StubExecutor.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/common/test/src/main/java/com/redhat/thermostat/testutils/StubExecutor.java Thu Jun 30 10:20:27 2016 -0400 @@ -0,0 +1,73 @@ +/* + * Copyright 2012-2016 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * . + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.testutils; + +import java.util.Collections; +import java.util.List; +import java.util.concurrent.AbstractExecutorService; +import java.util.concurrent.TimeUnit; + +public class StubExecutor extends AbstractExecutorService { + @Override + public void shutdown() { + } + + @Override + public List shutdownNow() { + return Collections.emptyList(); + } + + @Override + public boolean isShutdown() { + return false; + } + + @Override + public boolean isTerminated() { + return false; + } + + @Override + public boolean awaitTermination(long l, TimeUnit timeUnit) throws InterruptedException { + return false; + } + + @Override + public void execute(Runnable runnable) { + runnable.run(); + } +} \ No newline at end of file diff -r df9c589c6e0c -r cbf98b5f488d vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpDetailsController.java --- a/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpDetailsController.java Thu Jun 30 08:43:12 2016 -0400 +++ b/vm-heap-analysis/client-core/src/main/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpDetailsController.java Thu Jun 30 10:20:27 2016 -0400 @@ -87,14 +87,8 @@ public void setDump(HeapDump dump) { this.heapDump = dump; - ObjectHistogram histogram = null; - try { - histogram = dump.getHistogram(); - } catch (IOException e) { - log.log(Level.SEVERE, "unexpected error while reading heap dump", e); - } - + ObjectHistogram histogram = readHistogram(); Objects.requireNonNull(histogram); heapHistogramView = histogramViewProvider.createView(); @@ -125,17 +119,33 @@ dump.searchObjects("A_RANDOM_PATTERN", 1); } + private ObjectHistogram readHistogram() { + try { + return heapDump.getHistogram(); + } catch (IOException e) { + log.log(Level.SEVERE, "unexpected error while reading heap dump", e); + return null; + } + } + private void searchForObject(final String searchText) { - + appService.getApplicationExecutor().execute(new Runnable() { @Override public void run() { - Collection objectIds = heapDump.wildcardSearch(searchText); - - ObjectHistogram toDisplay = new ObjectHistogram(); - for (String id : objectIds) { - JavaHeapObject heapObject = heapDump.findObject(id); - toDisplay.addThing(heapObject); + ObjectHistogram toDisplay; + if (searchText == null || searchText.trim().isEmpty()) { + toDisplay = readHistogram(); + if (toDisplay == null) { + return; + } + } else { + toDisplay = new ObjectHistogram(); + Collection objectIds = heapDump.wildcardSearch(searchText); + for (String id : objectIds) { + JavaHeapObject heapObject = heapDump.findObject(id); + toDisplay.addThing(heapObject); + } } heapHistogramView.setHistogram(toDisplay); } diff -r df9c589c6e0c -r cbf98b5f488d vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpDetailsControllerTest.java --- a/vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpDetailsControllerTest.java Thu Jun 30 08:43:12 2016 -0400 +++ b/vm-heap-analysis/client-core/src/test/java/com/redhat/thermostat/vm/heap/analysis/client/core/internal/HeapDumpDetailsControllerTest.java Thu Jun 30 10:20:27 2016 -0400 @@ -36,22 +36,10 @@ package com.redhat.thermostat.vm.heap.analysis.client.core.internal; -import junit.framework.Assert; - -import static junit.framework.Assert.assertTrue; -import static org.mockito.Matchers.anyInt; -import static org.mockito.Matchers.isA; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import java.io.IOException; - -import org.junit.After; -import org.junit.Before; -import org.junit.Test; - +import com.redhat.thermostat.common.ActionEvent; +import com.redhat.thermostat.common.ActionListener; import com.redhat.thermostat.common.ApplicationService; +import com.redhat.thermostat.testutils.StubExecutor; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapDumpDetailsView; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapDumpDetailsViewProvider; import com.redhat.thermostat.vm.heap.analysis.client.core.HeapHistogramView; @@ -64,15 +52,39 @@ import com.redhat.thermostat.vm.heap.analysis.client.core.ObjectRootsViewProvider; import com.redhat.thermostat.vm.heap.analysis.common.HeapDump; import com.redhat.thermostat.vm.heap.analysis.common.ObjectHistogram; +import com.sun.tools.hat.internal.model.JavaClass; +import com.sun.tools.hat.internal.model.JavaHeapObject; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; + +import java.io.IOException; +import java.util.Collections; + +import static junit.framework.Assert.fail; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.isA; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class HeapDumpDetailsControllerTest { private HeapDumpDetailsView view; private HeapDumpDetailsViewProvider viewProvider; private HeapHistogramViewProvider histogramProvider; + private HeapHistogramView histogramView; private HeapTreeMapViewProvider treeMapProvider; private ObjectDetailsViewProvider objectDetailsProvider; private ObjectRootsViewProvider objectRootsProvider; + private ApplicationService appService; + + private HeapDumpDetailsController controller; @Before public void setUp() { @@ -80,7 +92,7 @@ view = mock(HeapDumpDetailsView.class); when(viewProvider.createView()).thenReturn(view); - HeapHistogramView histogramView = mock(HeapHistogramView.class); + histogramView = mock(HeapHistogramView.class); histogramProvider = mock(HeapHistogramViewProvider.class); when(histogramProvider.createView()).thenReturn(histogramView); @@ -95,36 +107,24 @@ ObjectRootsView objectRootsView = mock(ObjectRootsView.class); objectRootsProvider = mock(ObjectRootsViewProvider.class); when(objectRootsProvider.createView()).thenReturn(objectRootsView); + + appService = mock(ApplicationService.class); + when(appService.getApplicationExecutor()).thenReturn(new StubExecutor()); + + controller = new HeapDumpDetailsController(appService, viewProvider, histogramProvider, treeMapProvider, + objectDetailsProvider, objectRootsProvider); } - @After - public void tearDown() { - viewProvider = null; - histogramProvider = null; - objectDetailsProvider = null; - objectRootsProvider = null; - } - - @Test + @Test(expected = NullPointerException.class) public void testSetDumpFailsWithEmptyDump() throws IOException { - HeapDumpDetailsController controller = setupController(); - HeapDump emptyDump = mock(HeapDump.class); when(emptyDump.getHistogram()).thenReturn(null); - boolean caught = false; - try { - controller.setDump(emptyDump); - } catch (NullPointerException e) { - caught = true; - } - assertTrue("Null pointer exception expected", caught); + controller.setDump(emptyDump); } @Test public void testSetDumpWorksWithValidDump() throws IOException { - HeapDumpDetailsController controller = setupController(); - HeapDump dump = mock(HeapDump.class); ObjectHistogram histogram = mock(ObjectHistogram.class); when(dump.getHistogram()).thenReturn(histogram); @@ -132,7 +132,7 @@ try { controller.setDump(dump); } catch (NullPointerException e) { - Assert.fail("Did not expect null pointer exception"); + fail("Did not expect null pointer exception"); } verify(dump).searchObjects(isA(String.class), anyInt()); @@ -140,11 +140,62 @@ isA(HeapTreeMapView.class)); } - private HeapDumpDetailsController setupController() { - ApplicationService appService = mock(ApplicationService.class); - return new HeapDumpDetailsController( - appService, viewProvider, histogramProvider, treeMapProvider, - objectDetailsProvider, objectRootsProvider); + @Test + public void testEmptySearchStringDisplaysFullHistogram() throws IOException { + HeapDump dump = mock(HeapDump.class); + ObjectHistogram histogram = mock(ObjectHistogram.class); + when(dump.getHistogram()).thenReturn(histogram); + + controller.setDump(dump); + verify(histogramView).setHistogram(histogram); + + ArgumentCaptor captor = + ArgumentCaptor.forClass(ActionListener.class); + verify(histogramView).addHistogramActionListener(captor.capture()); + ActionListener listener = captor.getValue(); + + ActionEvent actionEvent = new ActionEvent<>(this, HeapHistogramView.HistogramAction.SEARCH); + actionEvent.setPayload(""); + listener.actionPerformed(actionEvent); + + verify(histogramView, times(2)).setHistogram(histogram); + + verify(dump, never()).wildcardSearch(anyString()); + verify(dump, never()).findObject(anyString()); + } + + @Test + public void testNonEmptySearchStringDisplaysFilteredHistogram() throws IOException { + HeapDump dump = mock(HeapDump.class); + ObjectHistogram histogram = mock(ObjectHistogram.class); + String objectId = "objectId"; + when(dump.getHistogram()).thenReturn(histogram); + when(dump.wildcardSearch(anyString())).thenReturn(Collections.singleton(objectId)); + JavaHeapObject javaHeapObject = mock(JavaHeapObject.class); + when(javaHeapObject.getClazz()).thenReturn(mock(JavaClass.class)); + when(dump.findObject(anyString())).thenReturn(javaHeapObject); + + controller.setDump(dump); + verify(histogramView).setHistogram(histogram); + + ArgumentCaptor captor = + ArgumentCaptor.forClass(ActionListener.class); + verify(histogramView).addHistogramActionListener(captor.capture()); + ActionListener listener = captor.getValue(); + + ActionEvent actionEvent = new ActionEvent<>(this, HeapHistogramView.HistogramAction.SEARCH); + String searchTerm = "SEARCH_TERM"; + actionEvent.setPayload(searchTerm); + listener.actionPerformed(actionEvent); + + verify(histogramView, times(1)).setHistogram(histogram); // verify full histogram still displayed only once + verify(dump).wildcardSearch(searchTerm); + verify(dump).findObject(objectId); + + ArgumentCaptor histogramCaptor = ArgumentCaptor.forClass(ObjectHistogram.class); + verify(histogramView, times(2)).setHistogram(histogramCaptor.capture()); + ObjectHistogram objectHistogram = histogramCaptor.getValue(); + assertThat(objectHistogram.getHistogram().size(), is(1)); } }