From 3ac7ecdb13b24ab8a8da23dc95ee9b617bdc48e9 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Wed, 15 Jun 2022 12:29:13 -0400 Subject: [PATCH] GP-2184 - removed JMockit usage from some tests (part 4) --- .../app/plugin/core/gotoquery/GoToHelper.java | 6 +- ...AbstractGhidraHeadlessIntegrationTest.java | 31 ++++---- ...actDecompilerFindReferencesActionTest.java | 70 +++++++++++++++---- .../decompile/DecompilerNavigationTest.java | 58 ++++++++------- 4 files changed, 107 insertions(+), 58 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/gotoquery/GoToHelper.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/gotoquery/GoToHelper.java index 875a416a3c..e5a54beb51 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/gotoquery/GoToHelper.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/gotoquery/GoToHelper.java @@ -190,7 +190,7 @@ public class GoToHelper { * @return true if navigation was successful or a list of possible linkage locations * was displayed. */ - private boolean goToExternalLinkage(Navigatable nav, ExternalLocation externalLoc, + protected boolean goToExternalLinkage(Navigatable nav, ExternalLocation externalLoc, boolean popupAllowed) { if (externalLoc == null) { return false; @@ -457,10 +457,6 @@ public class GoToHelper { private Program findGoToProgram(Program currentProgram, Address address) { // we need to try and find a suitable program Program goToProgram = findProgramContaining(currentProgram, address); - if (goToProgram == null) { - return null; - } - return goToProgram; } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/test/AbstractGhidraHeadlessIntegrationTest.java b/Ghidra/Features/Base/src/main/java/ghidra/test/AbstractGhidraHeadlessIntegrationTest.java index 2934968a55..059ca01833 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/test/AbstractGhidraHeadlessIntegrationTest.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/test/AbstractGhidraHeadlessIntegrationTest.java @@ -15,7 +15,7 @@ */ package ghidra.test; -import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.*; import java.io.File; import java.io.IOException; @@ -115,7 +115,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock * Get the language and compiler spec associated with an old language name string. If the * language no longer exists, and suitable replacement language will be returned if found. If no * language is found, an exception will be thrown. - * + * * @param oldLanguageName old language name string * @return the language compiler and spec * @throws LanguageNotFoundException if the language is not found @@ -138,7 +138,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock /** * Creates an in-memory program with the given language - * + * * @param name the program name * @param languageString a language string of the format x86:LE:32:default * @param consumer a consumer for the program @@ -157,7 +157,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock /** * Creates an in-memory program with the given language - * + * * @param name the program name * @param languageString a language string of the format x86:LE:32:default * @param compilerSpecID the ID @@ -236,7 +236,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock /** * Provides a convenient method for modifying the current program, handling the transaction * logic and returning a result. - * + * * @param the return type * @param the exception type * @param p the program @@ -308,7 +308,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock /** * Undo the last transaction on the domain object and wait for all events to be flushed. - * + * * @param dobj The domain object upon which to perform the undo. * @param wait if true, wait for undo to fully complete in Swing thread. If a modal dialog may * result from this undo, wait should be set false. @@ -332,7 +332,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock /** * Redo the last undone transaction on the domain object and wait for all events to be flushed. - * + * * @param dobj The domain object upon which to perform the redo. * @param wait if true, wait for redo to fully complete in Swing thread. If a modal dialog may * result from this redo, wait should be set false. @@ -356,7 +356,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock /** * Undo the last transaction on the domain object and wait for all events to be flushed. - * + * * @param dobj The domain object upon which to perform the undo. */ public static void undo(final UndoableDomainObject dobj) { @@ -365,7 +365,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock /** * Redo the last undone transaction on domain object and wait for all events to be flushed. - * + * * @param dobj The domain object upon which to perform the redo. */ public static void redo(final UndoableDomainObject dobj) { @@ -375,7 +375,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock /** * Undo the last 'count' transactions on the domain object and wait for all events to be * flushed. - * + * * @param dobj The domain object upon which to perform the undo. * @param count number of transactions to undo */ @@ -388,7 +388,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock /** * Redo the last 'count' undone transactions on the domain object and wait for all events to be * flushed. - * + * * @param dobj The domain object upon which to perform the redo. * @param count number of transactions to redo */ @@ -514,7 +514,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock * *

* Do not leave this call in your test when committing changes. - * + * * @param p the program * @param address the address * @@ -601,6 +601,11 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock } } + T instance = tool.getService(service); + if (instance != null) { + serviceManager.removeService(service, instance); + } + set.add(replacement.getClass()); serviceManager.addService(service, replacement); @@ -614,7 +619,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock /** * Get language service used for testing. - * + * * @return language service. */ public synchronized static LanguageService getLanguageService() { diff --git a/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/AbstractDecompilerFindReferencesActionTest.java b/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/AbstractDecompilerFindReferencesActionTest.java index 0f03b1e62e..40b7700f49 100644 --- a/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/AbstractDecompilerFindReferencesActionTest.java +++ b/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/AbstractDecompilerFindReferencesActionTest.java @@ -17,6 +17,8 @@ package ghidra.app.plugin.core.decompile; import static org.junit.Assert.*; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; @@ -29,14 +31,16 @@ import ghidra.app.actions.AbstractFindReferencesDataTypeAction; import ghidra.app.actions.AbstractFindReferencesToAddressAction; import ghidra.app.nav.Navigatable; import ghidra.app.plugin.core.decompile.actions.FindReferencesToHighSymbolAction; +import ghidra.app.plugin.core.navigation.FindAppliedDataTypesService; import ghidra.app.plugin.core.navigation.locationreferences.LocationReferencesProvider; import ghidra.app.plugin.core.navigation.locationreferences.LocationReferencesService; import ghidra.app.services.*; import ghidra.program.model.data.DataType; import ghidra.program.model.listing.Program; import ghidra.program.util.ProgramLocation; +import ghidra.util.HelpLocation; import ghidra.util.task.TaskMonitor; -import mockit.*; +import mockit.Mock; public abstract class AbstractDecompilerFindReferencesActionTest extends AbstractDecompilerTest { @@ -44,8 +48,7 @@ public abstract class AbstractDecompilerFindReferencesActionTest extends Abstrac protected DockingActionIf findReferencesToSymbolAction; protected DockingActionIf findReferencesToAddressAction; - protected SpyDataTypeReferenceFinder spyReferenceFinder; - protected SpyLocationReferencesService spyLocationReferenceService; + protected SpyLocationReferencesService spyLocationReferenceService; @Override @Before @@ -63,18 +66,37 @@ public abstract class AbstractDecompilerFindReferencesActionTest extends Abstrac private void installSpyDataTypeReferenceFinder() { - spyReferenceFinder = new SpyDataTypeReferenceFinder<>(); - replaceService(tool, DataTypeReferenceFinder.class, new StubDataTypeReferenceFinder()); + // + // We replace services in order for us to spy on system internals. This gets tricky + // because the LocationReferencesPlugin installs 2 services. So, when we replace it, we + // lose both of it services. We only want to replace one of the services, so we have to + // re-install the plugin to be the service provider for the service we still need. + // + FindAppliedDataTypesService fadService = tool.getService(FindAppliedDataTypesService.class); + replaceService(tool, DataTypeReferenceFinder.class, new SpyDataTypeReferenceFinder()); - spyLocationReferenceService = new SpyLocationReferencesService<>(); + LocationReferencesService lrService = tool.getService(LocationReferencesService.class); + spyLocationReferenceService = new SpyLocationReferencesService(lrService); + replaceService(tool, LocationReferencesService.class, spyLocationReferenceService); + + // as noted above, put back this service implementation + replaceService(tool, FindAppliedDataTypesService.class, fadService); } protected void assertFindAllReferencesToCompositeFieldWasCalled() { + + int last = SpyDataTypeReferenceFinder.instances.size() - 1; + SpyDataTypeReferenceFinder spyReferenceFinder = + SpyDataTypeReferenceFinder.instances.get(last); assertEquals(1, spyReferenceFinder.getFindCompositeFieldReferencesCallCount()); assertEquals(0, spyReferenceFinder.getFindDataTypeReferencesCallCount()); } protected void assertFindAllReferencesToDataTypeWasCalled() { + + int last = SpyDataTypeReferenceFinder.instances.size() - 1; + SpyDataTypeReferenceFinder spyReferenceFinder = + SpyDataTypeReferenceFinder.instances.get(last); assertEquals(0, spyReferenceFinder.getFindCompositeFieldReferencesCallCount()); assertEquals(1, spyReferenceFinder.getFindDataTypeReferencesCallCount()); } @@ -152,11 +174,21 @@ public abstract class AbstractDecompilerFindReferencesActionTest extends Abstrac findReferencesAction.isAddToPopup(context)); } - public class SpyDataTypeReferenceFinder extends MockUp { + public static class SpyDataTypeReferenceFinder implements DataTypeReferenceFinder { + + private static List instances = new ArrayList<>(); private AtomicInteger dataTypeReferencesCallCount = new AtomicInteger(); private AtomicInteger compositeFieldReferencesCallCount = new AtomicInteger(); + public SpyDataTypeReferenceFinder() { + + // Instances of this class are created by ReferenceUtils via the ClassSearcher. Save + // the instances so we can use the spy in the test + instances.add(this); + } + + @Override @Mock public void findReferences(Program p, DataType dataType, Consumer callback, TaskMonitor monitor) { @@ -164,6 +196,7 @@ public abstract class AbstractDecompilerFindReferencesActionTest extends Abstrac dataTypeReferencesCallCount.incrementAndGet(); } + @Override @Mock public void findReferences(Program p, DataType dt, String fieldName, Consumer callback, TaskMonitor monitor) { @@ -171,6 +204,7 @@ public abstract class AbstractDecompilerFindReferencesActionTest extends Abstrac compositeFieldReferencesCallCount.incrementAndGet(); } + @Override @Mock public void findReferences(Program p, FieldMatcher fieldMatcher, Consumer callback, TaskMonitor monitor) { @@ -193,20 +227,30 @@ public abstract class AbstractDecompilerFindReferencesActionTest extends Abstrac } } - public class SpyLocationReferencesService - extends MockUp { + public class SpyLocationReferencesService implements LocationReferencesService { private AtomicInteger showReferencesCallCount = new AtomicInteger(); + private LocationReferencesService lrService; - @Mock - public void showReferencesToLocation(Invocation invocation, ProgramLocation location, - Navigatable navigatable) { + public SpyLocationReferencesService(LocationReferencesService lrService) { + this.lrService = lrService; + } + + @Override + public void showReferencesToLocation(ProgramLocation location, Navigatable navigatable) { showReferencesCallCount.incrementAndGet(); - invocation.proceed(location, navigatable); + lrService.showReferencesToLocation(location, navigatable); } public int getShowReferencesCallCount() { return showReferencesCallCount.get(); } + + @Override + public HelpLocation getHelpLocation() { + return null; + } + } + } diff --git a/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerNavigationTest.java b/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerNavigationTest.java index 2f5ee8d530..5544ea5bfd 100644 --- a/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerNavigationTest.java +++ b/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerNavigationTest.java @@ -28,13 +28,14 @@ import ghidra.app.plugin.core.codebrowser.CodeViewerProvider; import ghidra.app.plugin.core.gotoquery.GoToHelper; import ghidra.app.plugin.core.navigation.NavigationOptions; import ghidra.app.plugin.core.navigation.NextPrevAddressPlugin; +import ghidra.app.services.GoToService; +import ghidra.app.util.navigation.GoToServiceImpl; import ghidra.program.model.address.Address; import ghidra.program.model.listing.*; import ghidra.program.model.symbol.*; import ghidra.program.util.OperandFieldLocation; import ghidra.program.util.ProgramLocation; import ghidra.test.ClassicSampleX86ProgramBuilder; -import mockit.*; public class DecompilerNavigationTest extends AbstractDecompilerTest { @@ -49,6 +50,23 @@ public class DecompilerNavigationTest extends AbstractDecompilerTest { tool.showComponentProvider(cbProvider, true); } + private void installSpyGoToHelper() { + GoToHelper spyGoToHelper = new GoToHelper(tool) { + + @Override + protected boolean goToExternalLinkage(Navigatable nav, ExternalLocation externalLoc, + boolean popupAllowed) { + + goToExternalLinkageCalled = true; + return super.goToExternalLinkage(nav, externalLoc, popupAllowed); + } + }; + + GoToServiceImpl goToServiceImpl = (GoToServiceImpl) tool.getService(GoToService.class); + setInstanceField("helper", goToServiceImpl, spyGoToHelper); + + } + @Override protected Program getProgram() throws Exception { return buildProgram(); @@ -69,10 +87,10 @@ public class DecompilerNavigationTest extends AbstractDecompilerTest { public void testNavigation_ExternalEventDoesNotTriggerNavigation() { // - // Test to make sure that external ProgramLocationEvent notifications to not trigger - // the Decompiler to broadcast a new event. Setup a tool with the Listing and + // Test to make sure that external ProgramLocationEvent notifications to not trigger + // the Decompiler to broadcast a new event. Setup a tool with the Listing and // the Decompiler open. Then, navigate in the Listing and verify the address does not - // move. (This is somewhat subject to the Code Unit at the address in how the + // move. (This is somewhat subject to the Code Unit at the address in how the // Decompiler itself responds to the incoming event.) // @@ -94,8 +112,7 @@ public class DecompilerNavigationTest extends AbstractDecompilerTest { public void testFunctionNavigation_ExternalProgramFunction_OptionNavigateToExternal() throws Exception { - // this call triggers jMockit to load our spy - new SpyGoToHelper(); + installSpyGoToHelper(); tool.getOptions("Navigation") .setEnum("External Navigation", @@ -107,7 +124,7 @@ public class DecompilerNavigationTest extends AbstractDecompilerTest { // /* - 01005a32 e8 be d2 CALL ghidra + 01005a32 e8 be d2 CALL ghidra ff ff */ @@ -130,8 +147,7 @@ public class DecompilerNavigationTest extends AbstractDecompilerTest { public void testFunctionNavigation_ExternalProgramFunction_OptionNavigateToLinkage() throws Exception { - // this call triggers jMockit to load our spy - new SpyGoToHelper(); + installSpyGoToHelper(); tool.getOptions("Navigation") .setEnum("External Navigation", @@ -143,7 +159,7 @@ public class DecompilerNavigationTest extends AbstractDecompilerTest { // /* - 01005a32 e8 be d2 CALL ghidra + 01005a32 e8 be d2 CALL ghidra ff ff */ @@ -178,15 +194,14 @@ public class DecompilerNavigationTest extends AbstractDecompilerTest { } @Test - public void testFunctionNavigation_WithAViewThatCachesTheLastValidFunction() - throws Exception { + public void testFunctionNavigation_WithAViewThatCachesTheLastValidFunction() throws Exception { // // This is testing the case where the user starts on a function foo(). Ancillary windows - // will display tool, such as a decompiled view. Now, if the user clicks to a - // non-function location, such as data, the ancillary window may still show foo(), even + // will display tool, such as a decompiled view. Now, if the user clicks to a + // non-function location, such as data, the ancillary window may still show foo(), even // though the user is no longer in foo. At this point, if the user wishes to go to the - // previous function, then from the ancillary window's perspective, it is the function + // previous function, then from the ancillary window's perspective, it is the function // that came before foo(). // @@ -204,7 +219,7 @@ public class DecompilerNavigationTest extends AbstractDecompilerTest { provider.requestFocus(); waitForSwing(); - // + // // The Decompiler is focused, showing 'entry'. Going back while it is focused should go // to the function before 'entry', which is 'ghidra'. // @@ -282,15 +297,4 @@ public class DecompilerNavigationTest extends AbstractDecompilerTest { program.flushEvents(); waitForSwing(); } - - public class SpyGoToHelper extends MockUp { - - @Mock - private boolean goToExternalLinkage(Invocation invocation, Navigatable nav, - ExternalLocation externalLoc, boolean popupAllowed) { - - goToExternalLinkageCalled = true; - return invocation.proceed(nav, externalLoc, popupAllowed); - } - } }