From b807c4c9035699dcbe884d127d412db8210ac0ae Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Wed, 24 Dec 2014 20:22:46 +0100 Subject: [PATCH] remove test diff api delegations: all implementations use DiffHyperlink to store before/after/path so special interface to provide this information just create one level of delegation ensure that current diffHyperlink is preselected when multiple diff is open --- .../testframework/sm/runner/SMTestProxy.java | 9 +-- .../states/TestComparisionFailedState.java | 27 ++------- .../testframework/AbstractTestProxy.java | 22 ++----- .../actions/ViewAssertEqualsDiffAction.java | 60 ++++++++++++------- .../stacktrace/DiffHyperlink.java | 39 +++++++++--- .../intellij/execution/junit2/TestProxy.java | 10 ++-- .../junit2/states/ComparisonFailureState.java | 31 +--------- .../testng/model/TestProxy.java | 41 ++----------- 8 files changed, 98 insertions(+), 141 deletions(-) diff --git a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTestProxy.java b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTestProxy.java index 36ff59196ea4..24b3926c98aa 100644 --- a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTestProxy.java +++ b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTestProxy.java @@ -21,6 +21,7 @@ import com.intellij.execution.testframework.sm.SMStacktraceParser; import com.intellij.execution.testframework.sm.TestsLocationProviderUtil; import com.intellij.execution.testframework.sm.runner.states.*; import com.intellij.execution.testframework.sm.runner.ui.TestsPresentationUtil; +import com.intellij.execution.testframework.stacktrace.DiffHyperlink; import com.intellij.execution.ui.ConsoleViewContentType; import com.intellij.ide.util.EditSourceUtil; import com.intellij.openapi.application.ApplicationManager; @@ -550,15 +551,15 @@ public class SMTestProxy extends AbstractTestProxy { @Override @Nullable - public AssertEqualsDiffViewerProvider getDiffViewerProvider() { - if (myState instanceof AssertEqualsDiffViewerProvider) { - return (AssertEqualsDiffViewerProvider)myState; + public DiffHyperlink getDiffViewerProvider() { + if (myState instanceof TestComparisionFailedState) { + return ((TestComparisionFailedState)myState).getHyperlink(); } if (myChildren != null) { for (SMTestProxy child : myChildren) { if (!child.isDefect()) continue; - final AssertEqualsDiffViewerProvider provider = child.getDiffViewerProvider(); + final DiffHyperlink provider = child.getDiffViewerProvider(); if (provider != null) { return provider; } diff --git a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/states/TestComparisionFailedState.java b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/states/TestComparisionFailedState.java index 461001d3b111..dc5fef7166d0 100644 --- a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/states/TestComparisionFailedState.java +++ b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/states/TestComparisionFailedState.java @@ -30,7 +30,7 @@ import org.jetbrains.annotations.Nullable; /** * @author Roman.Chernyatchik */ -public class TestComparisionFailedState extends TestFailedState implements AbstractTestProxy.AssertEqualsMultiDiffViewProvider { +public class TestComparisionFailedState extends TestFailedState { private final String myErrorMsgPresentation; private final String myStacktracePresentation; private DiffHyperlink myHyperlink; @@ -64,27 +64,8 @@ public class TestComparisionFailedState extends TestFailedState implements Abstr printer.print(CompositePrintable.NEW_LINE, ConsoleViewContentType.ERROR_OUTPUT); } - public void openDiff(final Project project) { - myHyperlink.openDiff(project); - } - - @Override - public String getExpected() { - return myHyperlink.getLeft(); - } - - @Override - public String getActual() { - return myHyperlink.getRight(); - } - - @Override - public void openMultiDiff(Project project, AbstractTestProxy.AssertEqualsDiffChain chain) { - myHyperlink.openMultiDiff(project, chain); - } - - @Override - public String getFilePath() { - return myHyperlink.getFilePath(); + @Nullable + public DiffHyperlink getHyperlink() { + return myHyperlink; } } diff --git a/platform/testRunner/src/com/intellij/execution/testframework/AbstractTestProxy.java b/platform/testRunner/src/com/intellij/execution/testframework/AbstractTestProxy.java index 7b84c66ae2ac..d30295951ba8 100644 --- a/platform/testRunner/src/com/intellij/execution/testframework/AbstractTestProxy.java +++ b/platform/testRunner/src/com/intellij/execution/testframework/AbstractTestProxy.java @@ -21,6 +21,7 @@ package com.intellij.execution.testframework; import com.intellij.execution.Location; +import com.intellij.execution.testframework.stacktrace.DiffHyperlink; import com.intellij.openapi.actionSystem.DataKey; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Disposer; @@ -134,25 +135,14 @@ public abstract class AbstractTestProxy extends CompositePrintable { } @Nullable - public AssertEqualsDiffViewerProvider getDiffViewerProvider() { + public DiffHyperlink getDiffViewerProvider() { return null; } - public interface AssertEqualsDiffViewerProvider { - void openDiff(final Project project); - String getExpected(); - String getActual(); - } - public interface AssertEqualsDiffChain { - AssertEqualsMultiDiffViewProvider getPrevious(); - AssertEqualsMultiDiffViewProvider getCurrent(); - AssertEqualsMultiDiffViewProvider getNext(); - void setCurrent(AssertEqualsMultiDiffViewProvider provider); - } - - public interface AssertEqualsMultiDiffViewProvider extends AssertEqualsDiffViewerProvider { - void openMultiDiff(Project project, AssertEqualsDiffChain chain); - String getFilePath(); + DiffHyperlink getPrevious(); + DiffHyperlink getCurrent(); + DiffHyperlink getNext(); + void setCurrent(DiffHyperlink provider); } } diff --git a/platform/testRunner/src/com/intellij/execution/testframework/actions/ViewAssertEqualsDiffAction.java b/platform/testRunner/src/com/intellij/execution/testframework/actions/ViewAssertEqualsDiffAction.java index 36b9e2459ba3..a5615647ab6e 100644 --- a/platform/testRunner/src/com/intellij/execution/testframework/actions/ViewAssertEqualsDiffAction.java +++ b/platform/testRunner/src/com/intellij/execution/testframework/actions/ViewAssertEqualsDiffAction.java @@ -20,10 +20,13 @@ import com.intellij.execution.testframework.AbstractTestProxy; import com.intellij.execution.testframework.TestFrameworkRunningModel; import com.intellij.execution.testframework.TestTreeView; import com.intellij.execution.testframework.TestTreeViewAction; +import com.intellij.execution.testframework.stacktrace.DiffHyperlink; import com.intellij.openapi.actionSystem.*; import com.intellij.openapi.project.Project; import com.intellij.openapi.ui.Messages; +import com.intellij.openapi.util.Comparing; import org.jetbrains.annotations.NonNls; +import org.jetbrains.annotations.Nullable; import java.awt.*; import java.util.ArrayList; @@ -33,25 +36,23 @@ public class ViewAssertEqualsDiffAction extends AnAction implements TestTreeView @NonNls public static final String ACTION_ID = "openAssertEqualsDiff"; public void actionPerformed(final AnActionEvent e) { - if (!openDiff(e.getDataContext())) { + if (!openDiff(e.getDataContext(), null)) { final Component component = e.getData(PlatformDataKeys.CONTEXT_COMPONENT); Messages.showInfoMessage(component, "Comparison error was not found", "No Comparison Data Found"); } } - public static boolean openDiff(DataContext context) { + public static boolean openDiff(DataContext context, @Nullable DiffHyperlink currentHyperlink) { final AbstractTestProxy testProxy = AbstractTestProxy.DATA_KEY.getData(context); if (testProxy != null) { - final AbstractTestProxy.AssertEqualsDiffViewerProvider diffViewerProvider = testProxy.getDiffViewerProvider(); + DiffHyperlink diffViewerProvider = testProxy.getDiffViewerProvider(); if (diffViewerProvider != null) { final Project project = CommonDataKeys.PROJECT.getData(context); - if (diffViewerProvider instanceof AbstractTestProxy.AssertEqualsMultiDiffViewProvider) { - final TestFrameworkRunningModel runningModel = TestTreeView.MODEL_DATA_KEY.getData(context); - final List providers = collectAvailableProviders(runningModel); - final MyAssertEqualsDiffChain diffChain = - providers.size() > 1 ? new MyAssertEqualsDiffChain(providers, (AbstractTestProxy.AssertEqualsMultiDiffViewProvider)diffViewerProvider) : null; - ((AbstractTestProxy.AssertEqualsMultiDiffViewProvider)diffViewerProvider).openMultiDiff(project, diffChain); - } else { + final List providers = collectAvailableProviders(TestTreeView.MODEL_DATA_KEY.getData(context)); + if (providers.size() > 1) { + new MyAssertEqualsDiffChain(providers, diffViewerProvider, currentHyperlink).openMultiDiff(project); + } + else { diffViewerProvider.openDiff(project); } return true; @@ -60,16 +61,16 @@ public class ViewAssertEqualsDiffAction extends AnAction implements TestTreeView return false; } - private static List collectAvailableProviders(TestFrameworkRunningModel model) { - final List providers = new ArrayList(); + private static List collectAvailableProviders(TestFrameworkRunningModel model) { + final List providers = new ArrayList(); if (model != null) { final AbstractTestProxy root = model.getRoot(); final List allTests = root.getAllTests(); for (AbstractTestProxy test : allTests) { if (test.isLeaf()) { - final AbstractTestProxy.AssertEqualsDiffViewerProvider provider = test.getDiffViewerProvider(); - if (provider instanceof AbstractTestProxy.AssertEqualsMultiDiffViewProvider) { - providers.add((AbstractTestProxy.AssertEqualsMultiDiffViewProvider)provider); + final DiffHyperlink provider = test.getDiffViewerProvider(); + if (provider != null) { + providers.add(provider); } } } @@ -108,35 +109,48 @@ public class ViewAssertEqualsDiffAction extends AnAction implements TestTreeView private static class MyAssertEqualsDiffChain implements AbstractTestProxy.AssertEqualsDiffChain { - private final List myProviders; - private AbstractTestProxy.AssertEqualsMultiDiffViewProvider myProvider; + private final List myProviders; + private DiffHyperlink myProvider; - public MyAssertEqualsDiffChain(List providers, - AbstractTestProxy.AssertEqualsMultiDiffViewProvider provider) { + public MyAssertEqualsDiffChain(List providers, + DiffHyperlink provider, + DiffHyperlink hyperlink) { myProviders = providers; + if (hyperlink != null) { + for (DiffHyperlink viewProvider : providers) { + if (Comparing.equal(hyperlink, viewProvider)) { + provider = viewProvider; + break; + } + } + } myProvider = provider; } @Override - public AbstractTestProxy.AssertEqualsMultiDiffViewProvider getPrevious() { + public DiffHyperlink getPrevious() { final int prevIdx = (myProviders.size() + myProviders.indexOf(myProvider) - 1) % myProviders.size(); return myProviders.get(prevIdx); } @Override - public AbstractTestProxy.AssertEqualsMultiDiffViewProvider getCurrent() { + public DiffHyperlink getCurrent() { return myProvider; } @Override - public AbstractTestProxy.AssertEqualsMultiDiffViewProvider getNext() { + public DiffHyperlink getNext() { final int nextIdx = (myProviders.indexOf(myProvider) + 1) % myProviders.size(); return myProviders.get(nextIdx); } @Override - public void setCurrent(AbstractTestProxy.AssertEqualsMultiDiffViewProvider provider) { + public void setCurrent(DiffHyperlink provider) { myProvider = provider; } + + public void openMultiDiff(Project project) { + myProvider.openMultiDiff(project, this); + } } } diff --git a/platform/testRunner/src/com/intellij/execution/testframework/stacktrace/DiffHyperlink.java b/platform/testRunner/src/com/intellij/execution/testframework/stacktrace/DiffHyperlink.java index 995a56eea597..4846c338c104 100644 --- a/platform/testRunner/src/com/intellij/execution/testframework/stacktrace/DiffHyperlink.java +++ b/platform/testRunner/src/com/intellij/execution/testframework/stacktrace/DiffHyperlink.java @@ -29,6 +29,7 @@ import com.intellij.execution.ui.ConsoleViewContentType; import com.intellij.icons.AllIcons; import com.intellij.ide.DataManager; import com.intellij.openapi.actionSystem.*; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.diff.*; import com.intellij.openapi.project.Project; import com.intellij.openapi.vfs.LocalFileSystem; @@ -42,6 +43,7 @@ import java.io.File; public class DiffHyperlink implements Printable { private static final String NEW_LINE = "\n"; + private static final Logger LOG = Logger.getInstance("#" + DiffHyperlink.class.getName()); protected final String myExpected; protected final String myActual; @@ -97,7 +99,7 @@ public class DiffHyperlink implements Printable { } @Override - protected AbstractTestProxy.AssertEqualsMultiDiffViewProvider getNextId() { + protected DiffHyperlink getNextId() { return chain.getPrevious(); } }); @@ -107,7 +109,7 @@ public class DiffHyperlink implements Printable { } @Override - protected AbstractTestProxy.AssertEqualsMultiDiffViewProvider getNextId() { + protected DiffHyperlink getNextId() { return chain.getNext(); } }); @@ -135,11 +137,12 @@ public class DiffHyperlink implements Printable { @Override public void actionPerformed(@NotNull AnActionEvent e) { final DiffViewer viewer = e.getData(PlatformDataKeys.DIFF_VIEWER); + LOG.assertTrue(viewer != null); final Project project = e.getData(CommonDataKeys.PROJECT); - final AbstractTestProxy.AssertEqualsMultiDiffViewProvider nextProvider = getNextId(); + final DiffHyperlink nextProvider = getNextId(); myChain.setCurrent(nextProvider); - final SimpleDiffRequest nextRequest = - createRequest(project, myChain, nextProvider.getFilePath(), nextProvider.getExpected(), nextProvider.getActual()); + final SimpleDiffRequest nextRequest = createRequest(project, myChain, + nextProvider.getFilePath(), nextProvider.getLeft(), nextProvider.getRight()); viewer.setDiffRequest(nextRequest); } @@ -150,7 +153,7 @@ public class DiffHyperlink implements Printable { e.getPresentation().setEnabled(project != null && viewer != null); } - protected abstract AbstractTestProxy.AssertEqualsMultiDiffViewProvider getNextId(); + protected abstract DiffHyperlink getNextId(); } protected String getTitle() { @@ -186,9 +189,31 @@ public class DiffHyperlink implements Printable { return string.indexOf('\n') != -1 || string.indexOf('\r') != -1; } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof DiffHyperlink)) return false; + + DiffHyperlink hyperlink = (DiffHyperlink)o; + + if (myActual != null ? !myActual.equals(hyperlink.myActual) : hyperlink.myActual != null) return false; + if (myExpected != null ? !myExpected.equals(hyperlink.myExpected) : hyperlink.myExpected != null) return false; + if (myFilePath != null ? !myFilePath.equals(hyperlink.myFilePath) : hyperlink.myFilePath != null) return false; + + return true; + } + + @Override + public int hashCode() { + int result = myExpected != null ? myExpected.hashCode() : 0; + result = 31 * result + (myActual != null ? myActual.hashCode() : 0); + result = 31 * result + (myFilePath != null ? myFilePath.hashCode() : 0); + return result; + } + public class DiffHyperlinkInfo implements HyperlinkInfo { public void navigate(final Project project) { - if (ViewAssertEqualsDiffAction.openDiff(DataManager.getInstance().getDataContext())) { + if (ViewAssertEqualsDiffAction.openDiff(DataManager.getInstance().getDataContext(), DiffHyperlink.this)) { return; } openDiff(project); diff --git a/plugins/junit/src/com/intellij/execution/junit2/TestProxy.java b/plugins/junit/src/com/intellij/execution/junit2/TestProxy.java index cc00d6666f17..9f7daa1deade 100644 --- a/plugins/junit/src/com/intellij/execution/junit2/TestProxy.java +++ b/plugins/junit/src/com/intellij/execution/junit2/TestProxy.java @@ -20,12 +20,14 @@ import com.intellij.execution.Location; import com.intellij.execution.junit2.events.*; import com.intellij.execution.junit2.info.MethodLocation; import com.intellij.execution.junit2.info.TestInfo; +import com.intellij.execution.junit2.states.ComparisonFailureState; import com.intellij.execution.junit2.states.IgnoredState; import com.intellij.execution.junit2.states.Statistics; import com.intellij.execution.junit2.states.TestState; import com.intellij.execution.testframework.AbstractTestProxy; import com.intellij.execution.testframework.Filter; import com.intellij.execution.testframework.TestConsoleProperties; +import com.intellij.execution.testframework.stacktrace.DiffHyperlink; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.pom.Navigatable; @@ -325,14 +327,14 @@ public class TestProxy extends AbstractTestProxy { @Override @Nullable - public AssertEqualsDiffViewerProvider getDiffViewerProvider() { - if (myState instanceof AssertEqualsDiffViewerProvider) { - return (AssertEqualsDiffViewerProvider)myState; + public DiffHyperlink getDiffViewerProvider() { + if (myState instanceof ComparisonFailureState) { + return ((ComparisonFailureState)myState).getHyperlink(); } for (TestProxy proxy : getChildren()) { if (!proxy.isDefect()) continue; - final AssertEqualsDiffViewerProvider provider = proxy.getDiffViewerProvider(); + final DiffHyperlink provider = proxy.getDiffViewerProvider(); if (provider != null) { return provider; } diff --git a/plugins/junit/src/com/intellij/execution/junit2/states/ComparisonFailureState.java b/plugins/junit/src/com/intellij/execution/junit2/states/ComparisonFailureState.java index c772936f5d20..8a036c7fa04a 100644 --- a/plugins/junit/src/com/intellij/execution/junit2/states/ComparisonFailureState.java +++ b/plugins/junit/src/com/intellij/execution/junit2/states/ComparisonFailureState.java @@ -17,14 +17,12 @@ package com.intellij.execution.junit2.states; import com.intellij.execution.junit2.segments.ObjectReader; -import com.intellij.execution.testframework.AbstractTestProxy; import com.intellij.execution.testframework.Printer; import com.intellij.execution.testframework.stacktrace.DiffHyperlink; import com.intellij.execution.ui.ConsoleViewContentType; -import com.intellij.openapi.project.Project; import org.jetbrains.annotations.NonNls; -public class ComparisonFailureState extends FaultyState implements AbstractTestProxy.AssertEqualsMultiDiffViewProvider { +public class ComparisonFailureState extends FaultyState { private DiffHyperlink myHyperlink; @NonNls protected static final String EXPECTED_VALUE_MESSAGE_TEXT = "expected:<"; @@ -48,30 +46,7 @@ public class ComparisonFailureState extends FaultyState implements AbstractTestP myHyperlink.printOn(printer); } - public String getExpected() { - return myHyperlink.getLeft(); - } - - public String getActual() { - return myHyperlink.getRight(); - } - - public void openDiff(final Project project) { - if (myHyperlink != null) myHyperlink.openDiff(project); - } - - @Override - public void openMultiDiff(Project project, AbstractTestProxy.AssertEqualsDiffChain chain) { - if (myHyperlink != null) { - myHyperlink.openMultiDiff(project, chain); - } - } - - @Override - public String getFilePath() { - if (myHyperlink != null) { - return myHyperlink.getFilePath(); - } - return null; + public DiffHyperlink getHyperlink() { + return myHyperlink; } } diff --git a/plugins/testng/src/com/theoryinpractice/testng/model/TestProxy.java b/plugins/testng/src/com/theoryinpractice/testng/model/TestProxy.java index 1a0f507436ab..82d7fc77aee8 100644 --- a/plugins/testng/src/com/theoryinpractice/testng/model/TestProxy.java +++ b/plugins/testng/src/com/theoryinpractice/testng/model/TestProxy.java @@ -28,6 +28,8 @@ import com.intellij.openapi.util.registry.Registry; import com.intellij.pom.Navigatable; import com.intellij.psi.*; import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.util.Function; +import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.Nullable; import org.testng.remote.strprotocol.MessageHelper; @@ -290,18 +292,18 @@ public class TestProxy extends AbstractTestProxy { } @Override - public AssertEqualsDiffViewerProvider getDiffViewerProvider() { + public DiffHyperlink getDiffViewerProvider() { if (myHyperlink == null) { for (TestProxy proxy : getChildren()) { if (!proxy.isDefect()) continue; - final AssertEqualsDiffViewerProvider provider = proxy.getDiffViewerProvider(); + final DiffHyperlink provider = proxy.getDiffViewerProvider(); if (provider != null) { return provider; } } return null; } - return new MyAssertEqualsMultiDiffViewProvider(myHyperlink); + return myHyperlink; } private static String trimStackTrace(String stackTrace) { @@ -429,37 +431,4 @@ public class TestProxy extends AbstractTestProxy { return text; } } - - private static class MyAssertEqualsMultiDiffViewProvider implements AssertEqualsMultiDiffViewProvider { - private DiffHyperlink myHyperlink; - - public MyAssertEqualsMultiDiffViewProvider(DiffHyperlink hyperlink) { - myHyperlink = hyperlink; - } - - @Override - public void openDiff(Project project) { - myHyperlink.openDiff(project); - } - - @Override - public String getExpected() { - return myHyperlink.getLeft(); - } - - @Override - public String getActual() { - return myHyperlink.getRight(); - } - - @Override - public void openMultiDiff(Project project, AssertEqualsDiffChain chain) { - myHyperlink.openMultiDiff(project, chain); - } - - @Override - public String getFilePath() { - return myHyperlink.getFilePath(); - } - } }