From a7a1e540ceed45f087ec458db3da67cb58b09e04 Mon Sep 17 00:00:00 2001 From: Bart van Helvert Date: Thu, 8 Dec 2022 15:16:46 +0100 Subject: [PATCH] [test] Fix test diff document synchronization Also makes it so the update uses string content replacement instead of PSI literal replacement. GitOrigin-RevId: 43f8a0b5712e3807e1b0a043e33e2711d99412ef --- .../testframework/JavaTestDiffProvider.kt | 5 ---- .../messages/TestRunnerBundle.properties | 3 +- .../testframework/actions/TestDiffContent.kt | 22 +++++++------- .../actions/TestDiffProvider.java | 10 ------- .../actions/TestDiffRequestProcessor.java | 8 ++--- .../execution/junit/JavaTestDiffUpdateTest.kt | 29 +++++++++++++++++- .../execution/junit/JvmTestDiffUpdateTest.kt | 2 +- .../testIntegration/KotlinTestDiffProvider.kt | 11 ++----- .../KotlinTestDiffUpdateTest.kt | 30 +++++++++++++++++++ 9 files changed, 75 insertions(+), 45 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 7feceb6db1dc..aa1cb1390918 100644 --- a/java/execution/impl/src/com/intellij/execution/testframework/JavaTestDiffProvider.kt +++ b/java/execution/impl/src/com/intellij/execution/testframework/JavaTestDiffProvider.kt @@ -2,7 +2,6 @@ package com.intellij.execution.testframework import com.intellij.codeInsight.CodeInsightUtil -import com.intellij.openapi.project.Project import com.intellij.psi.* import com.intellij.util.asSafely import com.intellij.util.containers.ContainerUtil @@ -10,10 +9,6 @@ import com.siyeh.ig.testFrameworks.AssertHint.Companion.createAssertEqualsHint import org.jetbrains.uast.UMethod class JavaTestDiffProvider : JvmTestDiffProvider() { - override fun createActual(project: Project, element: PsiElement, actual: String): PsiElement { - return PsiElementFactory.getInstance(project).createExpressionFromText("\"$actual\"", element) - } - override fun getParamIndex(param: PsiElement): Int? { if (param is PsiParameter) { return param.parent.asSafely()?.parameters?.indexOf(param) diff --git a/platform/testRunner/resources/messages/TestRunnerBundle.properties b/platform/testRunner/resources/messages/TestRunnerBundle.properties index 3a077605bcfd..10b02800f574 100644 --- a/platform/testRunner/resources/messages/TestRunnerBundle.properties +++ b/platform/testRunner/resources/messages/TestRunnerBundle.properties @@ -43,5 +43,4 @@ default.package.presentable.name= test.config.first.pattern.and.few.more={0} and {1} more all.tests.scope.presentable.text=All Tests failed.to.start.message=Failed to start ''{0}'' -notification.group.test.runner=Test execution finished -test.diff.update.command=Update Expected With Actual \ No newline at end of file +notification.group.test.runner=Test execution finished \ No newline at end of file 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 006638127641..0c96b27a3597 100644 --- a/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffContent.kt +++ b/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffContent.kt @@ -6,12 +6,11 @@ import com.intellij.diff.actions.DocumentsSynchronizer import com.intellij.diff.actions.SynchronizedDocumentContent import com.intellij.diff.contents.DocumentContent import com.intellij.diff.util.DiffUserDataKeysEx -import com.intellij.execution.testframework.TestRunnerBundle -import com.intellij.openapi.command.WriteCommandAction import com.intellij.openapi.editor.event.DocumentEvent import com.intellij.openapi.fileTypes.FileType import com.intellij.openapi.fileTypes.FileTypes import com.intellij.openapi.project.Project +import com.intellij.psi.ElementManipulators import com.intellij.psi.PsiDocumentManager import com.intellij.psi.PsiElement import com.intellij.psi.SmartPsiElementPointer @@ -22,24 +21,24 @@ class TestDiffContent( private val project: Project, original: DocumentContent, text: String, - private val elemPtr: SmartPsiElementPointer, - private val createActual: (Project, PsiElement, String) -> PsiElement + private val elemPtr: SmartPsiElementPointer ) : SynchronizedDocumentContent(original) { override fun getContentType(): FileType = FileTypes.PLAIN_TEXT override val synchronizer: DocumentsSynchronizer = object : DocumentsSynchronizer(project, original.document, fakeDocument) { override fun onDocumentChanged1(event: DocumentEvent) { - replaceString(myDocument2, 0, myDocument2.textLength, event.newFragment) + PsiDocumentManager.getInstance(project).performForCommittedDocument(document1, Runnable { + val element = elemPtr.element ?: return@Runnable + replaceString(myDocument2, 0, myDocument2.textLength, ElementManipulators.getValueText(element)) + }) } override fun onDocumentChanged2(event: DocumentEvent) { if (!myDocument1.isWritable) return - val element = elemPtr.element ?: return try { myDuringModification = true - WriteCommandAction.runWriteCommandAction(project, TestRunnerBundle.message("test.diff.update.command"), null, Runnable { - element.replace(createActual(project, element, event.newFragment.toString())) - }) + val element = elemPtr.element ?: return + ElementManipulators.handleContentChange(element, event.document.text) } finally { myDuringModification = false } @@ -55,13 +54,12 @@ class TestDiffContent( fun create( project: Project, text: String, - elemPtr: SmartPsiElementPointer, - replacement: (Project, PsiElement, String) -> PsiElement + elemPtr: SmartPsiElementPointer ): TestDiffContent? { val element = elemPtr.element ?: return null val document = PsiDocumentManager.getInstance(project).getDocument(element.containingFile) ?: return null val diffContent = DiffContentFactory.getInstance().create(project, document) - return TestDiffContent(project, diffContent, text, elemPtr, replacement).apply { + return TestDiffContent(project, diffContent, text, elemPtr).apply { val originalLineConvertor = original.getUserData(DiffUserDataKeysEx.LINE_NUMBER_CONVERTOR) putUserData(DiffUserDataKeysEx.LINE_NUMBER_CONVERTOR, IntUnaryOperator { value -> if (!element.isValid) return@IntUnaryOperator - 1 diff --git a/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffProvider.java b/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffProvider.java index 323e7a47679d..fbcbb89e4949 100644 --- a/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffProvider.java +++ b/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffProvider.java @@ -5,7 +5,6 @@ import com.intellij.lang.LanguageExtension; import com.intellij.openapi.project.Project; import com.intellij.psi.PsiElement; import com.intellij.util.concurrency.annotations.RequiresReadLock; -import com.intellij.util.concurrency.annotations.RequiresWriteLock; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -23,13 +22,4 @@ public interface TestDiffProvider { @Nullable @RequiresReadLock PsiElement findExpected(@NotNull Project project, @NotNull String stackTrace); - - /** - * Creates the actual literal from text for replacement - * @param element The string literal element to replace - * @param actual The text used for the replacement - */ - @NotNull - @RequiresWriteLock - PsiElement createActual(@NotNull Project project, @NotNull PsiElement element, @NotNull String actual); } diff --git a/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffRequestProcessor.java b/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffRequestProcessor.java index 97c846b7720a..8795a4185993 100644 --- a/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffRequestProcessor.java +++ b/platform/testRunner/src/com/intellij/execution/testframework/actions/TestDiffRequestProcessor.java @@ -89,7 +89,7 @@ public class TestDiffRequestProcessor { PsiElement expected = ReadAction.compute(() -> getExpected(provider, testProxy)); if (expected != null) { file1 = ReadAction.compute(() -> PsiUtilCore.getVirtualFile(expected)); - content1 = ReadAction.compute(() -> createPsiDiffContent(provider, expected, text1)); + content1 = ReadAction.compute(() -> createPsiDiffContent(expected, text1)); } } } @@ -118,11 +118,9 @@ public class TestDiffRequestProcessor { return provider.findExpected(myProject, testProxy.getStacktrace()); } - private @Nullable DiffContent createPsiDiffContent(@NotNull TestDiffProvider provider, - @NotNull PsiElement element, - @NotNull String text) { + private @Nullable DiffContent createPsiDiffContent(@NotNull PsiElement element, @NotNull String text) { SmartPsiElementPointer elemPtr = SmartPointerManager.createPointer(element); - return TestDiffContent.Companion.create(myProject, text, elemPtr, provider::createActual); + return TestDiffContent.Companion.create(myProject, text, elemPtr); } @Override diff --git a/plugins/junit/test/com/intellij/execution/junit/JavaTestDiffUpdateTest.kt b/plugins/junit/test/com/intellij/execution/junit/JavaTestDiffUpdateTest.kt index d467a5090612..782efc30ef99 100644 --- a/plugins/junit/test/com/intellij/execution/junit/JavaTestDiffUpdateTest.kt +++ b/plugins/junit/test/com/intellij/execution/junit/JavaTestDiffUpdateTest.kt @@ -4,7 +4,6 @@ package com.intellij.execution.junit import org.intellij.lang.annotations.Language class JavaTestDiffUpdateTest : JvmTestDiffUpdateTest() { - @Suppress("SameParameterValue") private fun checkAcceptDiff( @Language("Java") before: String, @@ -103,6 +102,34 @@ class JavaTestDiffUpdateTest : JvmTestDiffUpdateTest() { """.trimIndent()) } + fun `test string literal diff with escape`() { + checkAcceptDiff(""" + import org.junit.Assert; + import org.junit.Test; + + public class MyJUnitTest { + @Test + public void testFoo() { + Assert.assertEquals("expected", "actual\""); + } + } + """.trimIndent(), """ + 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.assertEquals(Assert.java:117) + at org.junit.Assert.assertEquals(Assert.java:146) + at MyJUnitTest.testFoo(MyJUnitTest.java:7) + """.trimIndent()) + } + fun `test parameter reference diff`() { checkAcceptDiff(""" 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 34f89ccb625f..cf74103b0716 100644 --- a/plugins/junit/test/com/intellij/execution/junit/JvmTestDiffUpdateTest.kt +++ b/plugins/junit/test/com/intellij/execution/junit/JvmTestDiffUpdateTest.kt @@ -52,7 +52,7 @@ abstract class JvmTestDiffUpdateTest : JavaCodeInsightFixtureTestCase() { request.onAssigned(true) val document = request.contents.firstOrNull().asSafely()?.document!! document.setReadOnly(false) - WriteCommandAction.runWriteCommandAction(myFixture.project, Runnable { document.replaceString(0, 0, actual) }) + WriteCommandAction.runWriteCommandAction(myFixture.project, Runnable { document.replaceString(0, document.textLength, actual) }) assertEquals(after, myFixture.file.text) } } \ No newline at end of file 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 9e17156582ea..97d1b4aafe65 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 @@ -2,7 +2,6 @@ package org.jetbrains.kotlin.idea.testIntegration import com.intellij.execution.testframework.JvmTestDiffProvider -import com.intellij.openapi.project.Project import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile import com.intellij.util.asSafely @@ -16,12 +15,6 @@ import org.jetbrains.uast.UMethod import org.jetbrains.uast.toUElementOfType class KotlinTestDiffProvider : JvmTestDiffProvider() { - override fun createActual(project: Project, element: PsiElement, actual: String): PsiElement { - val factory = KtPsiFactory(project) - if (element is KtStringTemplateEntry) return factory.createLiteralStringTemplateEntry(actual) - return factory.createStringTemplate(actual) - } - override fun getParamIndex(param: PsiElement): Int? { if (param is KtParameter) { return param.parent.asSafely()?.parameters?.indexOf(param) @@ -45,8 +38,8 @@ class KotlinTestDiffProvider : JvmTestDiffProvider() { } else { call.valueArguments.getOrNull(argIndex)?.getArgumentExpression() ?: return null } - if (expr is KtStringTemplateExpression && expr.entries.size == 1) return expr.entries.first() - if (expr is KtStringTemplateEntry) return expr + if (expr is KtStringTemplateExpression && expr.entries.size == 1) return expr + if (expr is KtStringTemplateEntry) return expr.parent if (expr is KtNameReferenceExpression) { val resolved = expr.reference?.resolve() if (resolved is KtVariableDeclaration) { 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 a9853b44e8d2..30327cea3cea 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 @@ -46,6 +46,36 @@ class KotlinTestDiffUpdateTest : JvmTestDiffUpdateTest() { ) } + fun `test string literal diff with escape`() { + checkAcceptDiff( + """ + import org.junit.Assert + import org.junit.Test + + class MyJUnitTest { + @Test + fun testFoo() { + Assert.assertEquals("expected", "actual\"") + } + } + """.trimIndent(), """ + import org.junit.Assert + import org.junit.Test + + class MyJUnitTest { + @Test + fun testFoo() { + Assert.assertEquals("actual\"", "actual\"") + } + } + """.trimIndent(), "MyJUnitTest", "testFoo", "expected", "actual\"", """ + at org.junit.Assert.assertEquals(Assert.java:117) + at org.junit.Assert.assertEquals(Assert.java:146) + at MyJUnitTest.testFoo(MyJUnitTest.kt:7) + """.trimIndent() + ) + } + fun `test parameter reference diff`() { checkAcceptDiff( """