From 181d1cdcc086805ee30269caf05a2c05e1638bf4 Mon Sep 17 00:00:00 2001 From: Bart van Helvert Date: Wed, 18 Jan 2023 22:32:13 +0100 Subject: [PATCH] [test] IDEA-310675 Don't show code test diff when not asserting string expressions GitOrigin-RevId: a23fe5cb75ea5d545d55db20ed6793407abe2fdf --- .../testframework/JavaTestDiffProvider.kt | 7 ++- .../testframework/actions/TestDiffContent.kt | 5 +- plugins/junit/intellij.junit.iml | 1 + .../execution/junit/JavaTestDiffUpdateTest.kt | 51 +++++++++++++++++++ .../execution/junit/JvmTestDiffUpdateTest.kt | 14 +++++ .../testIntegration/KotlinTestDiffProvider.kt | 6 ++- .../KotlinTestDiffUpdateTest.kt | 50 ++++++++++++++++++ 7 files changed, 129 insertions(+), 5 deletions(-) diff --git a/java/execution/impl/src/com/intellij/execution/testframework/JavaTestDiffProvider.kt b/java/execution/impl/src/com/intellij/execution/testframework/JavaTestDiffProvider.kt index 209709920bee..e9f45f0c83a0 100644 --- a/java/execution/impl/src/com/intellij/execution/testframework/JavaTestDiffProvider.kt +++ b/java/execution/impl/src/com/intellij/execution/testframework/JavaTestDiffProvider.kt @@ -7,7 +7,7 @@ import com.intellij.psi.util.parentOfType import com.intellij.refactoring.suggested.endOffset import com.intellij.refactoring.suggested.startOffset import com.intellij.util.asSafely -import com.siyeh.ig.testFrameworks.AssertHint.Companion.createAssertEqualsHint +import com.siyeh.ig.testFrameworks.AssertHint import org.jetbrains.uast.UMethod import org.jetbrains.uast.UParameter @@ -34,7 +34,10 @@ class JavaTestDiffProvider : JvmTestDiffProvider() { override fun getExpected(call: PsiElement, param: UParameter?): PsiElement? { if (call !is PsiMethodCallExpression) return null val expr = if (param == null) { - createAssertEqualsHint(call)?.expected ?: return null + val assertHint = AssertHint.createAssertEqualsHint(call) ?: return null + if (assertHint.actual.type != PsiType.getJavaLangString(call.manager, call.resolveScope)) return null + if (assertHint.expected.type != PsiType.getJavaLangString(call.manager, call.resolveScope)) return null + assertHint.expected } else { val srcParam = param.sourcePsi?.asSafely() val paramList = srcParam?.parentOfType() diff --git a/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffContent.kt b/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffContent.kt index 0c96b27a3597..90aeddf21d66 100644 --- a/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffContent.kt +++ b/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffContent.kt @@ -38,8 +38,9 @@ class TestDiffContent( try { myDuringModification = true val element = elemPtr.element ?: return - ElementManipulators.handleContentChange(element, event.document.text) - } finally { + ElementManipulators.getManipulator(element)?.handleContentChange(element, event.document.text) + } + finally { myDuringModification = false } } diff --git a/plugins/junit/intellij.junit.iml b/plugins/junit/intellij.junit.iml index 8a282f93d708..a2b937570c7d 100644 --- a/plugins/junit/intellij.junit.iml +++ b/plugins/junit/intellij.junit.iml @@ -31,6 +31,7 @@ + diff --git a/plugins/junit/test/com/intellij/execution/junit/JavaTestDiffUpdateTest.kt b/plugins/junit/test/com/intellij/execution/junit/JavaTestDiffUpdateTest.kt index 4d39717e2e72..47f5e5fd037e 100644 --- a/plugins/junit/test/com/intellij/execution/junit/JavaTestDiffUpdateTest.kt +++ b/plugins/junit/test/com/intellij/execution/junit/JavaTestDiffUpdateTest.kt @@ -4,7 +4,18 @@ package com.intellij.execution.junit import com.intellij.openapi.editor.Document import org.intellij.lang.annotations.Language +@Suppress("AssertBetweenInconvertibleTypes", "NewClassNamingConvention") class JavaTestDiffUpdateTest : JvmTestDiffUpdateTest() { + @Suppress("SameParameterValue") + private fun checkHasNoDiff( + @Language("Java") before: String, + testClass: String, + testName: String, + expected: String, + actual: String, + stackTrace: String + ) = checkHasNoDiff(before, testClass, testName, expected, actual, stackTrace, "java") + @Suppress("SameParameterValue") private fun checkAcceptFullDiff( @Language("Java") before: String, @@ -85,6 +96,46 @@ class JavaTestDiffUpdateTest : JvmTestDiffUpdateTest() { """.trimIndent()) } + fun `test accept diff is not available when expected is not a string literal`() { + checkHasNoDiff(""" + import org.junit.Assert; + import org.junit.Test; + + public class MyJUnitTest { + @Test + public void testFoo() { + Assert.assertEquals(true, "actual"); + } + } + """.trimIndent(), "MyJUnitTest", "testFoo", "expected", "actual", """ + at org.junit.Assert.fail(Assert.java:89) + at org.junit.Assert.failNotEquals(Assert.java:835) + at org.junit.Assert.assertEquals(Assert.java:120) + at org.junit.Assert.assertEquals(Assert.java:146) + at MyJUnitTest.testFoo(MyJUnitTest.java:7) + """.trimIndent()) + } + + fun `test accept diff is not available when actual is not a string literal`() { + checkHasNoDiff(""" + import org.junit.Assert; + import org.junit.Test; + + public class MyJUnitTest { + @Test + public void testFoo() { + Assert.assertEquals("actual", actual); + } + } + """.trimIndent(), "MyJUnitTest", "testFoo", "expected", "actual", """ + at org.junit.Assert.fail(Assert.java:89) + at org.junit.Assert.failNotEquals(Assert.java:835) + at org.junit.Assert.assertEquals(Assert.java:120) + at org.junit.Assert.assertEquals(Assert.java:146) + at MyJUnitTest.testFoo(MyJUnitTest.java:7) + """.trimIndent()) + } + fun `test accept text block diff`() { checkAcceptFullDiff(""" import org.junit.Assert; diff --git a/plugins/junit/test/com/intellij/execution/junit/JvmTestDiffUpdateTest.kt b/plugins/junit/test/com/intellij/execution/junit/JvmTestDiffUpdateTest.kt index a3edd33984c0..a8ce454956aa 100644 --- a/plugins/junit/test/com/intellij/execution/junit/JvmTestDiffUpdateTest.kt +++ b/plugins/junit/test/com/intellij/execution/junit/JvmTestDiffUpdateTest.kt @@ -5,6 +5,7 @@ import com.intellij.diff.contents.DocumentContent import com.intellij.diff.requests.SimpleDiffRequest import com.intellij.execution.executors.DefaultRunExecutor import com.intellij.execution.testframework.JavaTestLocator +import com.intellij.execution.testframework.actions.TestDiffContent import com.intellij.execution.testframework.actions.TestDiffRequestProcessor import com.intellij.execution.testframework.sm.runner.MockRuntimeConfiguration import com.intellij.execution.testframework.sm.runner.SMTRunnerConsoleProperties @@ -58,6 +59,19 @@ abstract class JvmTestDiffUpdateTest : JavaCodeInsightFixtureTestCase() { setReadOnly(false) }!! + protected open fun checkHasNoDiff( + before: String, + testClass: String, + testName: String, + expected: String, + actual: String, + stackTrace: String, + fileExt: String + ) { + val request = createDiffRequest(before, testClass, testName, expected, actual, stackTrace, fileExt) + assertNull(request.contents.firstOrNull { it is TestDiffContent }) + } + protected open fun checkAcceptFullDiff( before: String, after: String, diff --git a/plugins/kotlin/idea/src/org/jetbrains/kotlin/idea/testIntegration/KotlinTestDiffProvider.kt b/plugins/kotlin/idea/src/org/jetbrains/kotlin/idea/testIntegration/KotlinTestDiffProvider.kt index a3b717180997..d19389964166 100644 --- a/plugins/kotlin/idea/src/org/jetbrains/kotlin/idea/testIntegration/KotlinTestDiffProvider.kt +++ b/plugins/kotlin/idea/src/org/jetbrains/kotlin/idea/testIntegration/KotlinTestDiffProvider.kt @@ -4,6 +4,7 @@ package org.jetbrains.kotlin.idea.testIntegration import com.intellij.execution.testframework.JvmTestDiffProvider import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile +import com.intellij.psi.PsiType import com.intellij.psi.util.parentOfType import com.intellij.util.asSafely import com.siyeh.ig.testFrameworks.UAssertHint @@ -35,7 +36,10 @@ class KotlinTestDiffProvider : JvmTestDiffProvider() { if (call !is KtCallExpression) return null val expr = if (param == null) { val uCallElement = call.toUElementOfType() ?: return null - UAssertHint.createAssertEqualsUHint(uCallElement)?.expected?.sourcePsi ?: return null + val assertHint = UAssertHint.createAssertEqualsUHint(uCallElement) ?: return null + if (assertHint.expected.getExpressionType() != PsiType.getJavaLangString(call.manager, call.resolveScope)) return null + if (assertHint.actual.getExpressionType() != PsiType.getJavaLangString(call.manager, call.resolveScope)) return null + assertHint.expected.sourcePsi ?: return null } else { val argument = call.valueArguments.firstOrNull {it.getArgumentName()?.asName?.asString() == param.name } ?: let { val srcParam = param.sourcePsi?.asSafely() diff --git a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/testIntegration/KotlinTestDiffUpdateTest.kt b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/testIntegration/KotlinTestDiffUpdateTest.kt index 0ae86c5484b1..410ae91781e8 100644 --- a/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/testIntegration/KotlinTestDiffUpdateTest.kt +++ b/plugins/kotlin/idea/tests/test/org/jetbrains/kotlin/idea/testIntegration/KotlinTestDiffUpdateTest.kt @@ -6,6 +6,16 @@ import com.intellij.openapi.editor.Document import org.intellij.lang.annotations.Language class KotlinTestDiffUpdateTest : JvmTestDiffUpdateTest() { + @Suppress("SameParameterValue") + private fun checkHasNoDiff( + @Language("kotlin") before: String, + testClass: String, + testName: String, + expected: String, + actual: String, + stackTrace: String + ) = checkHasNoDiff(before, testClass, testName, expected, actual, stackTrace, "kt") + @Suppress("SameParameterValue") private fun checkAcceptFullDiff( @Language("kotlin") before: String, @@ -60,6 +70,46 @@ class KotlinTestDiffUpdateTest : JvmTestDiffUpdateTest() { ) } + fun `test accept diff is not available when expected is not a string literal`() { + checkHasNoDiff(""" + import org.junit.Assert + import org.junit.Test + + class MyJunitTest { + @Test + fun testFoo() { + Assert.assertEquals(true, "actual") + } + } + """.trimIndent(), "MyJunitTest", "testFoo", "expected", "actual", """ + at org.junit.Assert.fail(Assert.java:89) + at org.junit.Assert.failNotEquals(Assert.java:835) + at org.junit.Assert.assertEquals(Assert.java:120) + at org.junit.Assert.assertEquals(Assert.java:146) + at MyJunitTest.testFoo(MyJunitTest.kt:7) + """.trimIndent()) + } + + fun `test accept diff is not available when actual is not a string literal`() { + checkHasNoDiff(""" + import org.junit.Assert + import org.junit.Test + + class MyJunitTest { + @Test + fun testFoo() { + Assert.assertEquals("expected", true) + } + } + """.trimIndent(), "MyJunitTest", "testFoo", "expected", "actual", """ + at org.junit.Assert.fail(Assert.java:89) + at org.junit.Assert.failNotEquals(Assert.java:835) + at org.junit.Assert.assertEquals(Assert.java:120) + at org.junit.Assert.assertEquals(Assert.java:146) + at MyJunitTest.testFoo(MyJunitTest.kt:7) + """.trimIndent()) + } + fun `test physical string literal change sync`() { checkPhysicalDiff(before = """ import org.junit.Assert