From 143822159bef2a808083ff525156d38504dbb708 Mon Sep 17 00:00:00 2001 From: "Ilya.Kazakevich" Date: Fri, 14 Dec 2018 02:05:40 +0300 Subject: [PATCH] PY-32773: Move code that closes all opened branches to java side. ==Python part: Test runners do not report suites, only tests with qualified names. To create tree, our code splits test names, and we also must explicitly close each branch. This was done on Python side using atexit, but several frameworks launch atexit explicitly. so we decided to move it to Java side. This change is also required to support multiprocess. ==Platform side: We added special callback that listener may implement to be called before onTestingFinished. We need it because onTestingFinished makrs unfinished tests as skipped, and we need to close them my our code. --- ...eralIdBasedToSMTRunnerEventsConvertor.java | 1 + .../sm/runner/GeneralTestEventsProcessor.java | 7 + .../GeneralToSMTRunnerEventsConvertor.java | 1 + .../sm/runner/SMTRunnerEventsAdapter.java | 3 + .../sm/runner/SMTRunnerEventsListener.java | 10 +- .../GradleTestsExecutionConsoleManager.java | 1 + python/helpers/pycharm/_jb_runner_tools.py | 12 -- .../python/testing/PySMTestProxyUtils.kt | 37 ++++++ .../PythonTRunnerConsoleProperties.java | 5 +- .../python/testing/PyTestMagnitudeTest.kt | 121 ++++++++++++++++++ 10 files changed, 183 insertions(+), 15 deletions(-) create mode 100644 python/src/com/jetbrains/python/testing/PySMTestProxyUtils.kt create mode 100644 python/testSrc/com/jetbrains/python/testing/PyTestMagnitudeTest.kt diff --git a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralIdBasedToSMTRunnerEventsConvertor.java b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralIdBasedToSMTRunnerEventsConvertor.java index 60217646cc27..ebff88ea7861 100644 --- a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralIdBasedToSMTRunnerEventsConvertor.java +++ b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralIdBasedToSMTRunnerEventsConvertor.java @@ -52,6 +52,7 @@ public class GeneralIdBasedToSMTRunnerEventsConvertor extends GeneralTestEventsP @Override public void onFinishTesting() { + fireOnBeforeTestingFinished(myTestsRootProxy); LOG.debug("onFinishTesting"); // has been already invoked! // We don't know whether process was destroyed by user diff --git a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralTestEventsProcessor.java b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralTestEventsProcessor.java index 2a4aa4d28fe9..381c5f870dd9 100644 --- a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralTestEventsProcessor.java +++ b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralTestEventsProcessor.java @@ -217,6 +217,13 @@ public abstract class GeneralTestEventsProcessor implements Disposable { } } + protected void fireOnBeforeTestingFinished(@NotNull SMTestProxy.SMRootTestProxy root) { + myEventPublisher.onBeforeTestingFinished(root); + for (SMTRunnerEventsListener adapter : myListenerAdapters) { + adapter.onBeforeTestingFinished(root); + } + } + // custom progress statistics /** diff --git a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralToSMTRunnerEventsConvertor.java b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralToSMTRunnerEventsConvertor.java index fa0e654e8043..ae5a69fc96bb 100644 --- a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralToSMTRunnerEventsConvertor.java +++ b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/GeneralToSMTRunnerEventsConvertor.java @@ -77,6 +77,7 @@ public class GeneralToSMTRunnerEventsConvertor extends GeneralTestEventsProcesso @Override public void onFinishTesting() { + fireOnBeforeTestingFinished(myTestsRootProxy); // has been already invoked! // We don't know whether process was destroyed by user // or it finished after all tests have been run diff --git a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTRunnerEventsAdapter.java b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTRunnerEventsAdapter.java index f40ebe7b6e6d..2fc9f2aa47ee 100644 --- a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTRunnerEventsAdapter.java +++ b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTRunnerEventsAdapter.java @@ -29,6 +29,9 @@ public class SMTRunnerEventsAdapter implements SMTRunnerEventsListener { @Override public void onTestsCountInSuite(final int count) {} + @Override + public void onBeforeTestingFinished(@NotNull SMTestProxy.SMRootTestProxy testsRoot) { } + @Override public void onTestStarted(@NotNull final SMTestProxy test) {} @Override diff --git a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTRunnerEventsListener.java b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTRunnerEventsListener.java index 4aceb529114d..a74b849add75 100644 --- a/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTRunnerEventsListener.java +++ b/platform/smRunner/src/com/intellij/execution/testframework/sm/runner/SMTRunnerEventsListener.java @@ -18,8 +18,16 @@ public interface SMTRunnerEventsListener { * @param testsRoot */ void onTestingStarted(@NotNull SMTestProxy.SMRootTestProxy testsRoot); + /** - * After test framework finish testing + * Called before {@link #onTestingFinished(SMTestProxy.SMRootTestProxy)} + */ + default void onBeforeTestingFinished(@NotNull SMTestProxy.SMRootTestProxy testsRoot) { + + } + /** + * After test framework finish testing. + * @see #onBeforeTestingFinished(SMTestProxy.SMRootTestProxy) * @param testsRoot */ void onTestingFinished(@NotNull SMTestProxy.SMRootTestProxy testsRoot); diff --git a/plugins/gradle/java/src/execution/test/runner/GradleTestsExecutionConsoleManager.java b/plugins/gradle/java/src/execution/test/runner/GradleTestsExecutionConsoleManager.java index df6cd0d89221..170756f71c83 100644 --- a/plugins/gradle/java/src/execution/test/runner/GradleTestsExecutionConsoleManager.java +++ b/plugins/gradle/java/src/execution/test/runner/GradleTestsExecutionConsoleManager.java @@ -136,6 +136,7 @@ public class GradleTestsExecutionConsoleManager else { testsRootNode.setFinished(); } + resultsViewer.onBeforeTestingFinished(testsRootNode); resultsViewer.onTestingFinished(testsRootNode); }); } diff --git a/python/helpers/pycharm/_jb_runner_tools.py b/python/helpers/pycharm/_jb_runner_tools.py index 50c99eae749a..42404d0ba24c 100644 --- a/python/helpers/pycharm/_jb_runner_tools.py +++ b/python/helpers/pycharm/_jb_runner_tools.py @@ -150,12 +150,6 @@ class _TreeManager(object): parent = self._get_node_id(self.parent_branch) if self.parent_branch else "0" return str(current), str(parent) - def close_all(self): - if not self.current_branch: - return None - return "close", self.current_branch - - TREE_MANAGER = _TreeManager() _old_service_messages = messages.TeamcityServiceMessages @@ -386,12 +380,6 @@ def jb_start_tests(): return namespace.path, namespace.target, additional_args -def _close_all_tests(): - NewTeamcityServiceMessages().close_all() - - -atexit.register(_close_all_tests) - def jb_doc_args(framework_name, args): """ diff --git a/python/src/com/jetbrains/python/testing/PySMTestProxyUtils.kt b/python/src/com/jetbrains/python/testing/PySMTestProxyUtils.kt new file mode 100644 index 000000000000..d4d898009363 --- /dev/null +++ b/python/src/com/jetbrains/python/testing/PySMTestProxyUtils.kt @@ -0,0 +1,37 @@ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.jetbrains.python.testing + +import com.intellij.execution.testframework.sm.runner.SMTestProxy +import com.intellij.execution.testframework.sm.runner.states.TestStateInfo + +fun SMTestProxy.calculateAndReturnMagnitude(): TestStateInfo.Magnitude { + if (!isLeaf) { + var hasSkippedChildren = false + var hasFailedChildren = false + var hasErrorChildren = false + var hasPassedChildren = false + var hasTerminated = false + + children.forEach { child -> + val childMagnitude = child.calculateAndReturnMagnitude() + when (childMagnitude) { + TestStateInfo.Magnitude.PASSED_INDEX -> hasPassedChildren = true + TestStateInfo.Magnitude.SKIPPED_INDEX, TestStateInfo.Magnitude.IGNORED_INDEX -> hasSkippedChildren = true + TestStateInfo.Magnitude.ERROR_INDEX -> hasErrorChildren = true + TestStateInfo.Magnitude.RUNNING_INDEX, TestStateInfo.Magnitude.TERMINATED_INDEX -> hasTerminated = true + TestStateInfo.Magnitude.FAILED_INDEX -> hasFailedChildren = true + else -> hasPassedChildren = true + } + } + + when { + hasTerminated -> setTerminated() + hasErrorChildren -> setTestFailed(TestStateInfo.Magnitude.ERROR_INDEX.title, null, true) + hasFailedChildren -> setTestFailed(TestStateInfo.Magnitude.FAILED_INDEX.title, null, false) + hasSkippedChildren && !hasPassedChildren -> setTestIgnored(null, null) + else -> setFinished() + } + } + + return magnitudeInfo +} \ No newline at end of file diff --git a/python/src/com/jetbrains/python/testing/PythonTRunnerConsoleProperties.java b/python/src/com/jetbrains/python/testing/PythonTRunnerConsoleProperties.java index 1ba04520e6f3..970591d8d000 100644 --- a/python/src/com/jetbrains/python/testing/PythonTRunnerConsoleProperties.java +++ b/python/src/com/jetbrains/python/testing/PythonTRunnerConsoleProperties.java @@ -70,12 +70,13 @@ public class PythonTRunnerConsoleProperties extends SMTRunnerConsoleProperties { private static final String EMPTY_SUITE = PyBundle.message("runcfg.tests.empty_suite"); @Override - public void onTestingFinished(@NotNull final SMTestProxy.SMRootTestProxy testsRoot) { - super.onTestingFinished(testsRoot); + public void onBeforeTestingFinished(@NotNull final SMTestProxy.SMRootTestProxy testsRoot) { if (testsRoot.isEmptySuite()) { testsRoot.setPresentation(EMPTY_SUITE); testsRoot.setTestFailed(EMPTY_SUITE, null, false); } + PySMTestProxyUtilsKt.calculateAndReturnMagnitude(testsRoot); + super.onBeforeTestingFinished(testsRoot); } @Override diff --git a/python/testSrc/com/jetbrains/python/testing/PyTestMagnitudeTest.kt b/python/testSrc/com/jetbrains/python/testing/PyTestMagnitudeTest.kt new file mode 100644 index 000000000000..1ab73ca5b199 --- /dev/null +++ b/python/testSrc/com/jetbrains/python/testing/PyTestMagnitudeTest.kt @@ -0,0 +1,121 @@ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.jetbrains.python.testing + +import com.intellij.execution.testframework.sm.runner.SMTestProxy +import com.intellij.execution.testframework.sm.runner.states.TestStateInfo +import com.jetbrains.env.PyAbstractTestProcessRunner +import org.junit.Assert +import org.junit.Before +import org.junit.Test + + +class PyTestMagnitudeTest { + + private lateinit var root: SMTestProxy.SMRootTestProxy + private val tree: String get() = PyAbstractTestProcessRunner.getFormattedTestTree(root) + + private val test11 = "test11" + + + @Before + fun setUp() { + root = SMTestProxy.SMRootTestProxy() + + val file1 = SMTestProxy("file1", false, null) + val test11 = SMTestProxy(test11, false, null) + val test12 = SMTestProxy("test12", false, null) + + val file2 = SMTestProxy("file2", false, null) + val test21 = SMTestProxy("test21", false, null) + val test22 = SMTestProxy("test22", false, null) + + file1.addChild(test11) + file1.addChild(test12) + + file2.addChild(test21) + file2.addChild(test22) + + root.addChild(file1) + root.addChild(file2) + } + + @Test + fun testSuccessPass() { + root.children.forEach { file -> + file.children.forEach { test -> + test.setStarted() + test.setFinished() + } + } + Assert.assertEquals(TestStateInfo.Magnitude.PASSED_INDEX, root.calculateAndReturnMagnitude()) + Assert.assertEquals("Test tree:\n" + + "[root](+)\n" + + ".file1(+)\n" + + "..test11(+)\n" + + "..test12(+)\n" + + ".file2(+)\n" + + "..test21(+)\n" + + "..test22(+)\n", tree) + } + + + @Test + fun testFailedOne() { + root.children.forEach { file -> + file.children.forEach { test -> + test.setStarted() + if (test.name == test11) test.setTestFailed("fail", null, false) else test.setFinished() + } + } + Assert.assertEquals(TestStateInfo.Magnitude.FAILED_INDEX, root.calculateAndReturnMagnitude()) + Assert.assertEquals("Test tree:\n" + + "[root](-)\n" + + ".file1(-)\n" + + "..test11(-)\n" + + "..test12(+)\n" + + ".file2(+)\n" + + "..test21(+)\n" + + "..test22(+)\n", tree) + } + + @Test + fun testTerminatedOne() { + root.children.forEach { file -> + file.children.forEach { test -> + test.setStarted() + if (test.name != test11) { + test.setFinished() + } + } + } + Assert.assertEquals(TestStateInfo.Magnitude.TERMINATED_INDEX, root.calculateAndReturnMagnitude()) + Assert.assertEquals("Test tree:\n" + + "[root][T]\n" + + ".file1[T]\n" + + "..test11[T]\n" + + "..test12(+)\n" + + ".file2(+)\n" + + "..test21(+)\n" + + "..test22(+)\n", tree) + } + + @Test + fun testIgnoredAll() { + root.children.forEach { file -> + file.children.forEach { test -> + test.setStarted() + test.setTestIgnored("for a good reason", null) + + } + } + Assert.assertEquals(TestStateInfo.Magnitude.IGNORED_INDEX, root.calculateAndReturnMagnitude()) + Assert.assertEquals("Test tree:\n" + + "[root](~)\n" + + ".file1(~)\n" + + "..test11(~)\n" + + "..test12(~)\n" + + ".file2(~)\n" + + "..test21(~)\n" + + "..test22(~)\n", tree) + } +}