From 25bfa2b6ad7daa6cb461c9b4424a697e017728d2 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Tue, 17 Sep 2019 15:53:13 -0400 Subject: [PATCH] Tests - fixes for file chooser timing issue --- .../filechooser/GhidraFileChooser.java | 18 +++++--- .../filechooser/GhidraFileChooserTest.java | 42 ++++++++----------- .../util/reflection/ReflectionUtilities.java | 23 +++++++++- 3 files changed, 51 insertions(+), 32 deletions(-) diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/filechooser/GhidraFileChooser.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/filechooser/GhidraFileChooser.java index 1d63a2ed7d..602a2d11da 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/filechooser/GhidraFileChooser.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/filechooser/GhidraFileChooser.java @@ -72,6 +72,7 @@ import util.HistoryList; public class GhidraFileChooser extends DialogComponentProvider implements GhidraFileChooserListener, FileFilter { + static final String UP_BUTTON_NAME = "UP_BUTTON"; private static final Color FOREROUND_COLOR = Color.BLACK; private static final Color BACKGROUND_COLOR = Color.WHITE; static final String PREFERENCES_PREFIX = "G_FILE_CHOOSER"; @@ -494,7 +495,7 @@ public class GhidraFileChooser extends DialogComponentProvider forwardButton.addActionListener(e -> goForward()); upLevelButton = new EmptyBorderButton(upIcon); - upLevelButton.setName("UP_BUTTON"); + upLevelButton.setName(UP_BUTTON_NAME); upLevelButton.setToolTipText("Up one level"); upLevelButton.addActionListener(e -> goUpOneDirectoryLevel()); @@ -800,12 +801,16 @@ public class GhidraFileChooser extends DialogComponentProvider // the current dir, then we need to update if (force || !directory.equals(currentDirectory)) { worker.schedule(new UpdateDirectoryContentsJob(directory, null, addToHistory)); + return; } - // we only get here if the given dir is the current directory and we are not forcing - // an update - else { - setSelectedFileAndUpdateDisplay((isFilesOnly() ? null : directory)); - } + + // we only get here if the new dir is the current dir and we are not forcing an update + // TODO this code causes unexpected behavior when in 'directories only' mode in that + // this will cause the current directory to change. The behavior can be seen by + // putting this code back in and then running the tests. No tests are failing with this + // code removed. We are leaving this code here for a couple releases in case we find + // a code path that requires it. + // setSelectedFileAndUpdateDisplay((isFilesOnly() ? null : directory)); } boolean pendingUpdate() { @@ -1012,6 +1017,7 @@ public class GhidraFileChooser extends DialogComponentProvider validatedFiles.setFile(null); initialFile = selectedFiles.getFile(); initialFileToSelect = initialFile; + SystemUtilities.runSwingLater(() -> { File selectedFile = selectedFiles.getFile(); if (!fileExists(selectedFile)) { diff --git a/Ghidra/Framework/Docking/src/test.slow/java/docking/widgets/filechooser/GhidraFileChooserTest.java b/Ghidra/Framework/Docking/src/test.slow/java/docking/widgets/filechooser/GhidraFileChooserTest.java index 1d8738dd63..7c961dec23 100644 --- a/Ghidra/Framework/Docking/src/test.slow/java/docking/widgets/filechooser/GhidraFileChooserTest.java +++ b/Ghidra/Framework/Docking/src/test.slow/java/docking/widgets/filechooser/GhidraFileChooserTest.java @@ -19,8 +19,7 @@ package docking.widgets.filechooser; import static docking.widgets.filechooser.GhidraFileChooserMode.*; -import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; import java.awt.Dimension; @@ -205,16 +204,16 @@ public class GhidraFileChooserTest extends AbstractDockingTest { @Test public void testUp() throws Exception { - final File testDir = new File(getTestDirectoryPath()); - setFile(testDir); + File testDir = new File(getTestDirectoryPath()); + setDir(testDir); - pressButtonByName(chooser.getComponent(), "UP_BUTTON"); + pressButtonByName(chooser.getComponent(), GhidraFileChooser.UP_BUTTON_NAME); // We set the selected file to be a dir, so the start directory is that dir's parent. We // hit the up button, which will move the dir up past that parent. - File expectedFile = testDir.getParentFile().getParentFile(); + File expectedFile = testDir.getParentFile(); assertEquals(expectedFile, getCurrentDirectory()); - pressButtonByName(chooser.getComponent(), "UP_BUTTON"); + pressButtonByName(chooser.getComponent(), GhidraFileChooser.UP_BUTTON_NAME); assertEquals(expectedFile.getParentFile(), getCurrentDirectory()); // now keep pressing up--it should fail eventually @@ -223,7 +222,7 @@ public class GhidraFileChooserTest extends AbstractDockingTest { while (upCount < magicStopValue) { upCount++; try { - pressButtonByName(chooser.getComponent(), "UP_BUTTON"); + pressButtonByName(chooser.getComponent(), GhidraFileChooser.UP_BUTTON_NAME); } catch (AssertionError e) { // good! @@ -456,7 +455,7 @@ public class GhidraFileChooserTest extends AbstractDockingTest { assertTrue(!newFile.equals(getCurrentDirectory())); } - /** + /* * The user should be able to enter a directory into the text field and navigate to that * directory by pressing Enter or OK. *

@@ -532,7 +531,7 @@ public class GhidraFileChooserTest extends AbstractDockingTest { FileUtilities.deleteDir(tempSubDir); } - /** + /* * The user should be able to enter a directory into the text field and navigate to that * directory by pressing Enter or OK. *

@@ -593,10 +592,8 @@ public class GhidraFileChooserTest extends AbstractDockingTest { chooser.isShowing()); // re-launch the closed chooser - runSwing(() -> { - chooser.setCurrentDirectory(absoluteTempDir); - chooser.show(); - }, false); + setDir(absoluteTempDir); + runSwing(() -> chooser.show(), false); waitForSwing(); waitForNewDirLoad(absoluteTempDir); @@ -611,13 +608,13 @@ public class GhidraFileChooserTest extends AbstractDockingTest { waitForChooser(); selectedDir = getSelectedFile(); assertEquals( - "Entering a relative dir path and pressing Enter did not " + "chooser that directory!", + "Entering a relative dir path and pressing Enter did not chooser that directory!", tempSubDir, selectedDir); assertFalse("File chooser did not close when selecting a valid, relative subdirectory!", chooser.isShowing()); } - /** + /* * The user should be able to enter a directory into the text field and navigate to that * directory by pressing Enter or OK. *

@@ -678,10 +675,8 @@ public class GhidraFileChooserTest extends AbstractDockingTest { !chooser.isShowing()); // re-launch the closed chooser - runSwing(() -> { - chooser.setCurrentDirectory(absoluteTempDir); - chooser.show(); - }, false); + setDir(absoluteTempDir); + runSwing(() -> chooser.show(), false); waitForSwing(); waitForNewDirLoad(absoluteTempDir); @@ -788,7 +783,7 @@ public class GhidraFileChooserTest extends AbstractDockingTest { assertEquals(file.getAbsolutePath(), selectedFile.getAbsolutePath()); } - /** + /* * Test that an update/refresh of the current directory does not interfere with the user's * editing of the text field, which may include the drop-down selection window. */ @@ -940,8 +935,7 @@ public class GhidraFileChooserTest extends AbstractDockingTest { // Force chooser into user's home directory as it always exists File testDir = getHomeDir(); - chooser.setCurrentDirectory(testDir); - waitForChooser(); + setDir(testDir); setMode(FILES_AND_DIRECTORIES); @@ -1097,7 +1091,7 @@ public class GhidraFileChooserTest extends AbstractDockingTest { chooser.isShowing()); } - /** + /* * NOTE: make sure this test matches the features described by * {@link GhidraFileChooser#setSelectedFile(File)} */ diff --git a/Ghidra/Framework/Utility/src/main/java/utilities/util/reflection/ReflectionUtilities.java b/Ghidra/Framework/Utility/src/main/java/utilities/util/reflection/ReflectionUtilities.java index 410ff374c0..b8c42ff9fa 100644 --- a/Ghidra/Framework/Utility/src/main/java/utilities/util/reflection/ReflectionUtilities.java +++ b/Ghidra/Framework/Utility/src/main/java/utilities/util/reflection/ReflectionUtilities.java @@ -27,6 +27,7 @@ public class ReflectionUtilities { private static final String JAVA_AWT_PATTERN = "java.awt"; private static final String JAVA_REFLECT_PATTERN = "java.lang.reflect"; + private static final String JDK_INTERNAL_REFLECT_PATTERN = "jdk.internal.reflect"; private static final String SWING_JAVA_PATTERN = "java.swing"; private static final String SWING_JAVAX_PATTERN = "javax.swing"; private static final String SUN_AWT_PATTERN = "sun.awt"; @@ -350,6 +351,23 @@ public class ReflectionUtilities { return filterJavaThrowable(t); } + /** + * A convenience method to create a throwable, filtering boiler-plate Java-related + * lines (e.g., AWT, Swing, Security, etc). + * This can be useful for emitting diagnostic stack traces with reduced noise. + * + *

This method differs from {@link #createJavaFilteredThrowable()} in that this method + * returns a String, which is useful when printing log messages without having to directly + * print the stack trace. + * + * @return the new throwable + */ + public static String createJavaFilteredThrowableString() { + Throwable t = createThrowableWithStackOlderThan(); + Throwable filtered = filterJavaThrowable(t); + return stackTraceToString(filtered); + } + /** * A convenience method to take a throwable, filter boiler-plate Java-related * lines (e.g., AWT, Swing, Security, etc). @@ -361,8 +379,9 @@ public class ReflectionUtilities { public static Throwable filterJavaThrowable(Throwable t) { StackTraceElement[] trace = t.getStackTrace(); StackTraceElement[] filtered = filterStackTrace(trace, JAVA_AWT_PATTERN, - JAVA_REFLECT_PATTERN, SWING_JAVA_PATTERN, SWING_JAVAX_PATTERN, SECURITY_PATTERN, - SUN_AWT_PATTERN, SUN_REFLECT_PATTERN, MOCKIT_PATTERN, JUNIT_PATTERN); + JAVA_REFLECT_PATTERN, JDK_INTERNAL_REFLECT_PATTERN, SWING_JAVA_PATTERN, + SWING_JAVAX_PATTERN, SECURITY_PATTERN, SUN_AWT_PATTERN, SUN_REFLECT_PATTERN, + MOCKIT_PATTERN, JUNIT_PATTERN); t.setStackTrace(filtered); return t; }