From c47c880379b8037aaf6d5eba374f9edb8d50cdb8 Mon Sep 17 00:00:00 2001 From: Bernd Hufmann Date: Thu, 26 May 2016 14:46:33 -0400 Subject: [PATCH] tmf: Bug 494689: fix failing name conflict handling in trace import This patch also adds test cases for RENAME_ALL and OVERWRITE_ALL in the corresponding SWTBot test class. It also fixes the test cases when overwriting existing resources without confirmation. Change-Id: I7b4a7779091d824502ea15a18573f162304d143b Signed-off-by: Bernd Hufmann Reviewed-on: https://git.eclipse.org/r/73759 Reviewed-by: Matthew Khouzam Tested-by: Jean-Christian Kouame Reviewed-by: Hudson CI Reviewed-by: Marc-Andre Laperle Tested-by: Marc-Andre Laperle --- .../tests/AbstractImportAndReadSmokeTest.java | 25 ++++++++ .../tests/StandardImportAndReadSmokeTest.java | 59 +++++++++++++++---- .../importtrace/ImportConflictHandler.java | 34 +++++++---- .../project/wizards/importtrace/Messages.java | 4 ++ .../wizards/importtrace/messages.properties | 1 + 5 files changed, 103 insertions(+), 20 deletions(-) diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/AbstractImportAndReadSmokeTest.java b/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/AbstractImportAndReadSmokeTest.java index 4a6a63e390..204fef13e8 100644 --- a/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/AbstractImportAndReadSmokeTest.java +++ b/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/AbstractImportAndReadSmokeTest.java @@ -37,6 +37,7 @@ import org.eclipse.swtbot.swt.finder.widgets.SWTBotText; import org.eclipse.swtbot.swt.finder.widgets.SWTBotToolbarButton; import org.eclipse.swtbot.swt.finder.widgets.SWTBotTree; import org.eclipse.swtbot.swt.finder.widgets.SWTBotTreeItem; +import org.eclipse.tracecompass.internal.tmf.ui.project.wizards.importtrace.ImportConfirmation; import org.eclipse.tracecompass.internal.tmf.ui.views.statistics.TmfStatisticsViewImpl; import org.eclipse.tracecompass.testtraces.ctf.CtfTestTrace; import org.eclipse.tracecompass.tmf.core.signal.TmfSelectionRangeUpdatedSignal; @@ -149,9 +150,33 @@ public abstract class AbstractImportAndReadSmokeTest { * Finishes the wizard */ protected void importFinish() { + importFinish(ImportConfirmation.CONTINUE); + } + + /** + * Finishes the wizard + * + * @param confirmationMode + * a confirmation value + * Note: Only {@link ImportConfirmation#RENAME_ALL}, + * {@link ImportConfirmation#OVERWRITE_ALL}, + * {@link ImportConfirmation#CONTINUE} are supported + */ + protected void importFinish(ImportConfirmation confirmationMode) { SWTBotShell shell = fBot.activeShell(); final SWTBotButton finishButton = fBot.button("Finish"); finishButton.click(); + if (confirmationMode == ImportConfirmation.RENAME_ALL) { + fBot.waitUntil(Conditions.shellIsActive("Confirmation")); + SWTBotShell shell2 = fBot.activeShell(); + SWTBotButton button = shell2.bot().button("Rename All"); + button.click(); + } else if (confirmationMode == ImportConfirmation.OVERWRITE_ALL) { + fBot.waitUntil(Conditions.shellIsActive("Confirmation")); + SWTBotShell shell2 = fBot.activeShell(); + SWTBotButton button = shell2.bot().button("Overwrite All"); + button.click(); + } fBot.waitUntil(Conditions.shellCloses(shell)); SWTBotUtils.waitForJobs(); } diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/StandardImportAndReadSmokeTest.java b/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/StandardImportAndReadSmokeTest.java index 7be0215c63..da529b4703 100644 --- a/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/StandardImportAndReadSmokeTest.java +++ b/ctf/org.eclipse.tracecompass.tmf.ctf.ui.swtbot.tests/src/org/eclipse/tracecompass/tmf/ctf/ui/swtbot/tests/StandardImportAndReadSmokeTest.java @@ -57,6 +57,7 @@ import org.eclipse.swtbot.swt.finder.widgets.SWTBotText; import org.eclipse.swtbot.swt.finder.widgets.SWTBotTree; import org.eclipse.swtbot.swt.finder.widgets.SWTBotTreeItem; import org.eclipse.swtbot.swt.finder.widgets.TimeoutException; +import org.eclipse.tracecompass.internal.tmf.ui.project.wizards.importtrace.ImportConfirmation; import org.eclipse.tracecompass.internal.tmf.ui.project.wizards.importtrace.ImportTraceWizard; import org.eclipse.tracecompass.internal.tmf.ui.project.wizards.importtrace.ImportTraceWizardPage; import org.eclipse.tracecompass.internal.tmf.ui.project.wizards.importtrace.Messages; @@ -156,7 +157,7 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe */ @Test public void testImportWithExperimentValidation() throws Exception { - testImport(ImportTraceWizardPage.OPTION_CREATE_EXPERIMENT, false, false, false); + testImport(ImportTraceWizardPage.OPTION_CREATE_EXPERIMENT, false, false, false, true, ImportConfirmation.CONTINUE); } /** @@ -190,10 +191,34 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe */ @Test public void testImportFromDirectoryOverwrite() throws Exception { - testImport(0, false, false); + testImport(0, false, false, true, false, ImportConfirmation.CONTINUE); testImport(ImportTraceWizardPage.OPTION_OVERWRITE_EXISTING_RESOURCES, false, false); } + /** + * Test import from directory, overwrite all + * + * @throws Exception + * on error + */ + @Test + public void testImportFromDirectoryOverwriteRenameAll() throws Exception { + testImport(0, false, false, true, false, ImportConfirmation.CONTINUE); + testImport(0, false, false, true, true, ImportConfirmation.RENAME_ALL); + } + + /** + * Test import from directory, overwrite all + * + * @throws Exception + * on error + */ + @Test + public void testImportFromDirectoryOverwriteOverwriteAll() throws Exception { + testImport(0, false, false, true, false, ImportConfirmation.CONTINUE); + testImport(0, false, false, true, true, ImportConfirmation.OVERWRITE_ALL); + } + /** * Test import from archive * @@ -233,7 +258,7 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe */ @Test public void testImportFromArchiveOverwrite() throws Exception { - testImport(0, false, true); + testImport(0, false, true, true, false, ImportConfirmation.CONTINUE); testImport(ImportTraceWizardPage.OPTION_OVERWRITE_EXISTING_RESOURCES, false, true); } @@ -426,10 +451,10 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe } private void testImport(int options, boolean testViews, boolean fromArchive) throws Exception { - testImport(options, testViews, fromArchive, true); + testImport(options, testViews, fromArchive, true, true, ImportConfirmation.CONTINUE); } - private void testImport(int options, boolean testViews, boolean fromArchive, boolean defaultExperiment) throws Exception { + private void testImport(int options, boolean testViews, boolean fromArchive, boolean defaultExperiment, boolean clearTraces, ImportConfirmation confirmationMode) throws Exception { String expectedSourceLocation = null; @NonNull String experimentName; @@ -464,14 +489,20 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe } checkFinishButton(true); - importFinish(); + importFinish(confirmationMode); IPath expectedElementPath = new Path(TRACE_NAME); if ((options & ImportTraceWizardPage.OPTION_PRESERVE_FOLDER_STRUCTURE) != 0) { expectedElementPath = new Path(TRACE_FOLDER).append(expectedElementPath); } - checkOptions(options, expectedSourceLocation, expectedElementPath, experimentName); + if (confirmationMode == ImportConfirmation.RENAME_ALL) { + IPath expectedElementPathRenamed = new Path(TRACE_NAME + "(2)"); + checkOptions(options, expectedSourceLocation, expectedElementPath, experimentName, expectedElementPathRenamed); + } else { + checkOptions(options, expectedSourceLocation, expectedElementPath, experimentName, null); + } + TmfEventsEditor tmfEd = SWTBotUtils.openEditor(fBot, TRACE_PROJECT_NAME, expectedElementPath); if (testViews) { testViews(tmfEd); @@ -480,7 +511,9 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe fBot.closeAllEditors(); SWTBotUtils.clearExperimentFolder(fBot, TRACE_PROJECT_NAME); - SWTBotUtils.clearTracesFolder(fBot, TRACE_PROJECT_NAME); + if (clearTraces) { + SWTBotUtils.clearTracesFolder(fBot, TRACE_PROJECT_NAME); + } } private void testImportAndExtractArchives(int options, boolean testViews, boolean fromArchive) throws Exception { @@ -731,10 +764,10 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe } private static void checkOptions(int optionFlags, String expectedSourceLocation, IPath expectedElementPath) throws CoreException { - checkOptions(optionFlags, expectedSourceLocation, expectedElementPath, null); + checkOptions(optionFlags, expectedSourceLocation, expectedElementPath, null, null); } - private static void checkOptions(int optionFlags, String expectedSourceLocation, IPath expectedElementPath, String experimentName) throws CoreException { + private static void checkOptions(int optionFlags, String expectedSourceLocation, IPath expectedElementPath, String experimentName, IPath expectedElementPathRenamed) throws CoreException { IProject project = getProjectResource(); assertTrue(project.exists()); TmfProjectElement tmfProject = TmfProjectRegistry.getProject(project, true); @@ -759,6 +792,12 @@ public class StandardImportAndReadSmokeTest extends AbstractImportAndReadSmokeTe IPath expectedPath = Path.ROOT.append(new Path(TRACE_PROJECT_NAME)).append(TmfTracesFolder.TRACES_FOLDER_NAME).append(expectedElementPath); assertEquals(expectedPath, traceResource.getFullPath()); + if (expectedElementPathRenamed != null) { + IPath expectedPathRenamed = Path.ROOT.append(new Path(TRACE_PROJECT_NAME)).append(TmfTracesFolder.TRACES_FOLDER_NAME).append(expectedElementPathRenamed); + IResource traceResourceRenamed = traces.get(1).getResource(); + assertEquals(expectedPathRenamed, traceResourceRenamed.getFullPath()); + } + String sourceLocation = traceResource.getPersistentProperty(TmfCommonConstants.SOURCE_LOCATION); assertNotNull(sourceLocation); assertEquals(expectedSourceLocation, sourceLocation); diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/ImportConflictHandler.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/ImportConflictHandler.java index 1c2f783ff6..687df41bfb 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/ImportConflictHandler.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/ImportConflictHandler.java @@ -15,6 +15,7 @@ import java.util.List; import org.eclipse.core.resources.IContainer; import org.eclipse.core.resources.IResource; +import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IProgressMonitor; @@ -118,7 +119,7 @@ public class ImportConflictHandler { // ------------------------------------------------------------------------ private ImportConfirmation checkForNameClash(IPath tracePath) throws InterruptedException { // handle rename - if (getExistingTrace(tracePath) != null) { + if (getExistingResource(tracePath) != null) { if ((fConfirmationMode == ImportConfirmation.RENAME_ALL) || (fConfirmationMode == ImportConfirmation.OVERWRITE_ALL) || (fConfirmationMode == ImportConfirmation.SKIP_ALL)) { @@ -138,7 +139,7 @@ public class ImportConflictHandler { private int promptForOverwrite(IPath tracePath) { final MessageDialog dialog = new MessageDialog(fShell, - null, null, NLS.bind(Messages.ImportTraceWizard_TraceAlreadyExists, tracePath.makeRelativeTo(fTraceFolderElement.getProject().getPath())), + Messages.ImportTraceWizard_MessageTitle, null, NLS.bind(Messages.ImportTraceWizard_TraceAlreadyExists, tracePath.makeRelativeTo(fTraceFolderElement.getProject().getPath())), MessageDialog.QUESTION, new String[] { ImportConfirmation.RENAME.getInName(), ImportConfirmation.RENAME_ALL.getInName(), @@ -164,18 +165,19 @@ public class ImportConflictHandler { return returnValue[0]; } - private String rename(IPath tracePath) { - TmfTraceElement trace = getExistingTrace(tracePath); - if (trace == null) { + private static String rename(IPath tracePath) { + IResource existingResource = getExistingResource(tracePath); + if (existingResource == null) { return tracePath.lastSegment(); } // Not using IFolder on purpose to leave the door open to import // directly into an IProject - IContainer folder = (IContainer) trace.getParent().getResource(); + IContainer folder = existingResource.getParent(); + int i = 2; while (true) { - String name = trace.getName() + '(' + Integer.toString(i++) + ')'; + String name = existingResource.getName() + '(' + Integer.toString(i++) + ')'; IResource resource = folder.findMember(name); if (resource == null) { return name; @@ -184,12 +186,19 @@ public class ImportConflictHandler { } private void delete(IPath tracePath, IProgressMonitor monitor) throws CoreException { - TmfTraceElement trace = getExistingTrace(tracePath); - if (trace == null) { + IResource existingResource = getExistingResource(tracePath); + if (existingResource == null) { + return; + } + TmfTraceElement existingTraceElement = getExistingTrace(tracePath); + if (existingTraceElement != null) { + // Delete existing TmfTraceElement + existingTraceElement.delete(monitor); return; } - trace.delete(monitor); + // Delete resource existing in workspace + existingResource.delete(true, monitor); } private TmfTraceElement getExistingTrace(IPath tracePath) { @@ -201,5 +210,10 @@ public class ImportConflictHandler { } return null; } + + private static IResource getExistingResource(IPath tracePath) { + // Look for existing resource + return ResourcesPlugin.getWorkspace().getRoot().findMember(tracePath); + } } diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/Messages.java b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/Messages.java index 8e1995ef8d..34128102e4 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/Messages.java +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/Messages.java @@ -83,6 +83,10 @@ public class Messages extends NLS { * The error message when a trace validation failed (import trace wizard). */ public static String ImportTraceWizard_TraceValidationFailed; + /** + * The title of message dialog (import trace wizard). + */ + public static String ImportTraceWizard_MessageTitle; /** * The error message when a trace already exists in project (import trace wizard). */ diff --git a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/messages.properties b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/messages.properties index a1af39e147..760cc146e2 100644 --- a/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/messages.properties +++ b/tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/internal/tmf/ui/project/wizards/importtrace/messages.properties @@ -26,6 +26,7 @@ ImportTraceWizard_CreateLinksInWorkspace=Create lin&ks in workspace ImportTraceWizard_PreserveFolderStructure=Preserve &folder structure ImportTraceWizard_CreateExperiment=Create experiment ImportTraceWizard_TraceValidationFailed=Validation failed for trace resource {0} +ImportTraceWizard_MessageTitle=Confirmation ImportTraceWizard_TraceAlreadyExists=Trace with name "{0}" already exists in project. Do you want to rename, overwrite or skip? ImportTraceWizard_ImportConfigurationRename=Rename ImportTraceWizard_ImportConfigurationRenameAll=Rename All -- 2.34.1