From bfe89551dec4e29c3b6ac6080ec57c5df599a98f Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Tue, 28 May 2019 17:34:32 -0400 Subject: [PATCH] Tests - fixed tests failing due to recent merges --- .../script/GhidraScriptComponentProvider.java | 69 +++++++++++-------- .../ghidra/app/script/GhidraScriptUtil.java | 19 +++-- .../AbstractGhidraScriptMgrPluginTest.java | 17 +++-- .../script/GhidraScriptMgrPlugin3Test.java | 9 +-- .../core/diff/DiffSaveSettingsTest.java | 6 +- .../java/docking/AbstractDockingTool.java | 5 +- .../actions/DockingToolActionManager.java | 11 ++- .../ghidra/util/task/AbstractTaskTest.java | 4 +- 8 files changed, 75 insertions(+), 65 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/script/GhidraScriptComponentProvider.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/script/GhidraScriptComponentProvider.java index 26286a5660..8dc4a894a2 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/script/GhidraScriptComponentProvider.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/script/GhidraScriptComponentProvider.java @@ -53,6 +53,7 @@ import ghidra.util.datastruct.WeakSet; import ghidra.util.table.GhidraTableFilterPanel; import ghidra.util.task.*; import resources.ResourceManager; +import util.CollectionUtils; import utilities.util.FileUtilities; public class GhidraScriptComponentProvider extends ComponentProviderAdapter { @@ -479,14 +480,6 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter { tableModel.fireTableDataChanged(); } - /* - * is more than just root node selected? - */ - boolean isSelectedCategory() { - TreePath path = scriptCategoryTree.getSelectionPath(); - return path != null && path.getPathCount() > 1; - } - String[] getSelectedCategoryPath() { TreePath currentPath = scriptCategoryTree.getSelectionPath(); @@ -528,7 +521,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter { tableModel.fireTableDataChanged(); - updateTreeNodesToReflectAvailableScripts(); + trimUnusedTreeCategories(); scriptRoot.fireNodeStructureChanged(scriptRoot); if (preRefreshSelectionPath != null) { @@ -537,6 +530,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter { } private void updateAvailableScriptFilesForAllPaths() { + List scriptsToRemove = tableModel.getScripts(); List scriptAccumulator = new ArrayList<>(); List dirPaths = pathManager.getPaths(); @@ -553,7 +547,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter { } GhidraScriptUtil.refreshDuplicates(); - refreshActions(); + refreshScriptData(); } private void updateAvailableScriptFilesForDirectory(List scriptsToRemove, @@ -577,7 +571,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter { } - private void refreshActions() { + private void refreshScriptData() { List scripts = tableModel.getScripts(); for (ResourceFile script : scripts) { @@ -593,26 +587,43 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter { } } - private void updateTreeNodesToReflectAvailableScripts() { - ArrayList nodesToRemove = new ArrayList<>(); - Iterator it = new BreadthFirstIterator(scriptCategoryTree, scriptRoot); - while (it.hasNext()) { - GTreeNode node = it.next(); - String[] category = getCategoryPath(node); - Iterator iter = GhidraScriptUtil.getScriptInfoIterator(); - boolean found = false; - while (iter.hasNext()) { - if (iter.next().isCategory(category)) { - found = true; - break; - } - } - if (!found) { - nodesToRemove.add(node); + // note: we really should just rebuild the tree instead of using this method + private void trimUnusedTreeCategories() { + + /* + Unusual Algorithm + + The tree nodes represent categories, but do not contain nodes for individual + scripts. We wish to remove any of the tree nodes that no longer represent script + categories. (This can happen when a script is deleted or its category is changed.) + This algorithm will assume that all nodes need to be deleted. Then, each script is + examined, using its category to mark a given node as 'safe'; that node's parents are + also marked as safe. Any nodes remaining unmarked have no reference script and + will be deleted. + */ + + // note: turn String[] to List to use hashing + Iterator scripts = GhidraScriptUtil.getScriptInfoIterator(); + Set> categories = new HashSet<>(); + for (ScriptInfo info : CollectionUtils.asIterable(scripts)) { + String[] path = info.getCategory(); + List category = Arrays.asList(path); + for (int i = 1; i <= category.size(); i++) { + categories.add(category.subList(0, i)); } } - for (GTreeNode node : nodesToRemove) { + List toDelete = new LinkedList<>(); + Iterator nodes = new BreadthFirstIterator(scriptCategoryTree, scriptRoot); + for (GTreeNode node : CollectionUtils.asIterable(nodes)) { + String[] path = getCategoryPath(node); + List category = Arrays.asList(path); + if (!categories.contains(category)) { + toDelete.add(node); + } + } + + for (GTreeNode node : toDelete) { GTreeNode parent = node.getParent(); if (parent != null) { parent.removeNode(node); @@ -874,7 +885,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter { private void updateCategoryTree(ResourceFile script) { scriptRoot.insert(script); - updateTreeNodesToReflectAvailableScripts(); + trimUnusedTreeCategories(); } private void buildFilter() { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/script/GhidraScriptUtil.java b/Ghidra/Features/Base/src/main/java/ghidra/app/script/GhidraScriptUtil.java index 7e5da97cc0..5649c3b08d 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/script/GhidraScriptUtil.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/script/GhidraScriptUtil.java @@ -206,7 +206,9 @@ public class GhidraScriptUtil { /** * Returns the output directory to which the given script file's generated .class file should - * be written. + * be written + * @param scriptFile the script file + * @return the directory */ public static ResourceFile getScriptCompileOutputDirectory(ResourceFile scriptFile) { return new ResourceFile(USER_SCRIPTS_BIN_DIR); @@ -349,8 +351,9 @@ public class GhidraScriptUtil { /** * Returns the list of directories to which scripts are compiled. + * @return the list * - * @see #getScriptCompileOutputDirectory(File) + * @see #getScriptCompileOutputDirectory(ResourceFile) */ public static List getScriptBinDirectories() { return Arrays.asList(new ResourceFile(USER_SCRIPTS_BIN_DIR)); @@ -667,7 +670,7 @@ public class GhidraScriptUtil { * scripts such that names are unique. If this method returns a non-null value, then the * name given name is taken. * - * @param name the name of the script for which to get a ScriptInfo + * @param scriptName the name of the script for which to get a ScriptInfo * @return a ScriptInfo matching the given name; null if no script by that name is known to * the script manager */ @@ -680,11 +683,13 @@ public class GhidraScriptUtil { } /** - * Runs the specified script with the specified state. + * Runs the specified script with the specified state * - * @param scriptState State representing environment variables that the script is able - * to access. - * @param script Script to be run. + * @param scriptState state representing environment variables that the script is able to access + * @param script Script to be run + * @param writer the writer to which warning and error messages will be written + * @param originator the client class requesting the script run; used for logging + * @param monitor the task monitor * @return whether the script successfully completed running */ public static boolean runScript(GhidraState scriptState, GhidraScript script, diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/AbstractGhidraScriptMgrPluginTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/AbstractGhidraScriptMgrPluginTest.java index e46f7702db..e53faacc07 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/AbstractGhidraScriptMgrPluginTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/AbstractGhidraScriptMgrPluginTest.java @@ -62,7 +62,8 @@ import ghidra.util.table.GhidraTableFilterPanel; import ghidra.util.task.*; import utilities.util.FileUtilities; -public abstract class AbstractGhidraScriptMgrPluginTest extends AbstractGhidraHeadedIntegrationTest { +public abstract class AbstractGhidraScriptMgrPluginTest + extends AbstractGhidraHeadedIntegrationTest { protected static final int MAX_TIME = 4000; protected static final int SCRIPT_TIMEOUT_SECS = 5; protected TestEnv env; @@ -190,6 +191,7 @@ public abstract class AbstractGhidraScriptMgrPluginTest extends AbstractGhidraHe protected void selectCategory(String category) { GTree categoryTree = (GTree) findComponentByName(provider.getComponent(), "CATEGORY_TREE"); + waitForTree(categoryTree); JTree jTree = (JTree) invokeInstanceMethod("getJTree", categoryTree); assertNotNull(jTree); GTreeNode child = categoryTree.getRootNode().getChild(category); @@ -345,6 +347,7 @@ public abstract class AbstractGhidraScriptMgrPluginTest extends AbstractGhidraHe * This call will: * -open the file in an editor * -update the text area and buffer fields of this test + * @param file the file to open */ protected void openInEditor(final ResourceFile file) { runSwing(() -> editor = provider.editScriptInGhidra(file)); @@ -500,13 +503,9 @@ public abstract class AbstractGhidraScriptMgrPluginTest extends AbstractGhidraHe buffer.append("Test text: ").append(testName.getMethodName()); runSwing(() -> { - - // TODO editorTextArea.requestFocusInWindow() editorTextArea.setText(buffer.toString()); }); - //typeText(buffer.toString()); - return buffer.toString(); } @@ -692,8 +691,6 @@ public abstract class AbstractGhidraScriptMgrPluginTest extends AbstractGhidraHe //@category name contents = contents.replaceFirst("//@category \\w+", "//@category " + newCategory); - Msg.debug(this, "new category string: " + newCategory); - writeStringToFile(script, contents); // @@ -703,7 +700,8 @@ public abstract class AbstractGhidraScriptMgrPluginTest extends AbstractGhidraHe // File file = script.getFile(false); long lastModified = file.lastModified(); - file.setLastModified(lastModified + (1000 * System.currentTimeMillis())); + long inTheFuture = 10000 + System.currentTimeMillis(); + file.setLastModified(lastModified + inTheFuture); return newCategory; } @@ -1318,7 +1316,8 @@ public abstract class AbstractGhidraScriptMgrPluginTest extends AbstractGhidraHe String parentClassName = parentScriptName.replaceAll("\\.java", ""); String importLine = (parentScriptPackage != null - ? ("import " + parentScriptPackage + "." + parentClassName + ";\n\n") : ""); + ? ("import " + parentScriptPackage + "." + parentClassName + ";\n\n") + : ""); //@formatter:off String newScript = diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/GhidraScriptMgrPlugin3Test.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/GhidraScriptMgrPlugin3Test.java index d9af8ccd6c..87465b1d7e 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/GhidraScriptMgrPlugin3Test.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/script/GhidraScriptMgrPlugin3Test.java @@ -41,10 +41,6 @@ import ghidra.util.exception.AssertException; public class GhidraScriptMgrPlugin3Test extends AbstractGhidraScriptMgrPluginTest { - public GhidraScriptMgrPlugin3Test() { - super(); - } - @Test public void testKeyBinding() throws Exception { @@ -209,6 +205,7 @@ public class GhidraScriptMgrPlugin3Test extends AbstractGhidraScriptMgrPluginTes String oldCategory = newCategory; newCategory = changeScriptCategory_WithSubcatogory(newScript); + refreshScriptManager(); assertCategoryInTree(newCategory); @@ -287,7 +284,7 @@ public class GhidraScriptMgrPlugin3Test extends AbstractGhidraScriptMgrPluginTes performAction(pathAction, false); waitForSwing(); - PickPathsDialog pathsDialog = env.waitForDialogComponent(PickPathsDialog.class, MAX_TIME); + PickPathsDialog pathsDialog = waitForDialogComponent(PickPathsDialog.class); final File dir = new File(getTestDirectoryPath() + "/test_scripts"); dir.mkdirs(); @@ -308,7 +305,7 @@ public class GhidraScriptMgrPlugin3Test extends AbstractGhidraScriptMgrPluginTes chooseJavaProvider(); - SaveDialog sd = env.waitForDialogComponent(SaveDialog.class, MAX_TIME); + SaveDialog sd = waitForDialogComponent(SaveDialog.class); final ListPanel listPanel = (ListPanel) findComponentByName(sd.getComponent(), "PATH_LIST"); assertNotNull(listPanel); diff --git a/Ghidra/Features/ProgramDiff/src/test.slow/java/ghidra/app/plugin/core/diff/DiffSaveSettingsTest.java b/Ghidra/Features/ProgramDiff/src/test.slow/java/ghidra/app/plugin/core/diff/DiffSaveSettingsTest.java index d1b21ee756..feb26c6366 100644 --- a/Ghidra/Features/ProgramDiff/src/test.slow/java/ghidra/app/plugin/core/diff/DiffSaveSettingsTest.java +++ b/Ghidra/Features/ProgramDiff/src/test.slow/java/ghidra/app/plugin/core/diff/DiffSaveSettingsTest.java @@ -15,7 +15,7 @@ */ package ghidra.app.plugin.core.diff; -import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.*; import java.awt.Window; @@ -35,10 +35,6 @@ import ghidra.test.TestEnv; public class DiffSaveSettingsTest extends DiffApplyTestAdapter { - public DiffSaveSettingsTest() { - super(); - } - @Override @Before public void setUp() throws Exception { diff --git a/Ghidra/Framework/Docking/src/main/java/docking/AbstractDockingTool.java b/Ghidra/Framework/Docking/src/main/java/docking/AbstractDockingTool.java index 96dab4b0b1..89bf204e42 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/AbstractDockingTool.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/AbstractDockingTool.java @@ -64,7 +64,10 @@ public abstract class AbstractDockingTool implements DockingTool { @Override public void removeComponentProvider(ComponentProvider provider) { - Runnable r = () -> winMgr.removeComponent(provider); + Runnable r = () -> { + actionMgr.removeComponentActions(provider); + winMgr.removeComponent(provider); + }; SystemUtilities.runSwingNow(r); } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/actions/DockingToolActionManager.java b/Ghidra/Framework/Docking/src/main/java/docking/actions/DockingToolActionManager.java index 414df4a7da..66030f504b 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/actions/DockingToolActionManager.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/actions/DockingToolActionManager.java @@ -170,7 +170,7 @@ public class DockingToolActionManager implements PropertyChangeListener { /** * Get all actions that have the action name which includes the action owner's name. * - * @param fullName full name for the action, e.g., "My Action (My Plugin)" + * @param fullActionName full name for the action, e.g., "My Action (My Plugin)" * @return list of actions; empty if no action exists with the given name */ public List getDockingActionsByFullActionName(String fullActionName) { @@ -248,11 +248,10 @@ public class DockingToolActionManager implements PropertyChangeListener { } /** - * Get the actions for the given provider and remove them from the - * actionMap; call the window manager to remove the provider. - * @param provider provider to be removed + * Get the actions for the given provider and remove them from the action map + * @param provider provider whose actions are to be removed */ - public void removeComponent(ComponentProvider provider) { + public void removeComponentActions(ComponentProvider provider) { Iterator iterator = winMgr.getComponentActions(provider); while (iterator.hasNext()) { DockingActionIf action = iterator.next(); @@ -262,7 +261,7 @@ public class DockingToolActionManager implements PropertyChangeListener { actionMap.remove(name); } } - winMgr.removeComponent(provider); + } @Override diff --git a/Ghidra/Framework/Docking/src/test/java/ghidra/util/task/AbstractTaskTest.java b/Ghidra/Framework/Docking/src/test/java/ghidra/util/task/AbstractTaskTest.java index 13434d6e86..d6e95fbccf 100644 --- a/Ghidra/Framework/Docking/src/test/java/ghidra/util/task/AbstractTaskTest.java +++ b/Ghidra/Framework/Docking/src/test/java/ghidra/util/task/AbstractTaskTest.java @@ -52,7 +52,7 @@ public class AbstractTaskTest extends AbstractDockingTest { TDEvent lastEvent = eventQueue.peekLast(); boolean swingIsLast = lastEvent.getThreadName().contains("AWT"); if (!swingIsLast) { - fail("The Swing thread did not block until the task finished"); + fail("The Swing thread did not block until the task finished.\nEvents: " + eventQueue); } } @@ -60,7 +60,7 @@ public class AbstractTaskTest extends AbstractDockingTest { TDEvent lastEvent = eventQueue.peekLast(); boolean swingIsLast = lastEvent.getThreadName().contains("AWT"); if (swingIsLast) { - fail("The Swing thread blocked until the task finished"); + fail("The Swing thread blocked until the task finished.\nEvents: " + eventQueue); } }