Implement infrastructure to check recorded trace #510

ivandev0 · 2025-02-10T11:45:08Z

This PR contains changes only about infrastructure. There are no new tests, only the example of how to write one. For now with each rerun we get a new trace (if I did everything right). We need to investigate it.

eupp · 2025-02-18T17:20:09Z

buildSrc/src/main/kotlin/IntegrationTestsTasks.kt

+    ProjectToTest("kotlinx.collections.immutable", "592f05fce02a1ad9e26cc6f3fdb55cdd97910599")
+)
+
+fun Project.registerIntegrationTestsTasks() {


Consider renaming to registerIntegrationTestsPrerequisites so that the name matches with the registered gradle task name.

eupp · 2025-02-18T17:20:51Z

buildSrc/src/main/kotlin/IntegrationTestsTasks.kt

+        }
+    }
+
+    tasks.register("integrationTestsPrerequisites") {


Should we rename to traceDebuggerIntegrationTestsPrerequisites?

eupp · 2025-02-18T17:21:28Z

buildSrc/src/main/kotlin/IntegrationTestsTasks.kt

+    tasks.register("integrationTestsPrerequisites") {
+        prerequisite.forEach { dependsOn(it) }
+    }
+}


Also, consider moving this file into TraceDebuggerTasks.kt since this functionality is related to the trace debugger specifically.

I renamed IntegrationTestsTasks file into TraceDebuggerIntegrationTestsTasks. I don't really want to push everything into one big file and it is better to separate test and release stuff.

buildSrc/src/main/kotlin/IntegrationTestsTasks.kt

eupp · 2025-02-18T17:25:07Z

src/jvm/main/org/jetbrains/kotlinx/lincheck/TraceDebugger.kt


-val isInTraceDebuggerMode = System.getProperty("lincheck.traceDebuggerMode", "false").toBoolean()
+private const val traceDebuggerModeProperty = "lincheck.traceDebuggerMode"


By convention in the Lincheck repo we usually use CAPITAL_CASE for constants.

eupp · 2025-02-18T17:42:53Z

src/jvm/test/org/jetbrains/kotlinx/lincheck_test/integration/AbstractIntegrationTest.kt

+            Assert.assertEquals(goldDataFile.readText(), tmpFile.readText())
+        } else {
+            copy(tmpFile, goldDataFile)
+            Assert.fail("The gold data file was created. Please rerun the test.")


Let's fail test in this case. To overwrite the expected test output, let's use a gradle property.
We already have one used for similar purposes in Lincheck representation tests, so we can re-use it here (it is called overwriteRepresentationTestsOutput).
See the links below:

lincheck/src/jvm/test/org/jetbrains/kotlinx/lincheck_test/util/TestUtils.kt

Line 54 in a758c54

if (OVERWRITE_REPRESENTATION_TESTS_OUTPUT) {

lincheck/src/jvm/test/org/jetbrains/kotlinx/lincheck_test/util/TestUtils.kt

Line 118 in a758c54

internal val OVERWRITE_REPRESENTATION_TESTS_OUTPUT: Boolean =

lincheck/gradle.properties

Line 22 in a758c54

overwriteRepresentationTestsOutput=false

What do you mean? The test will be failed with Assert.fail. This branch means that golden test data wasn't found and it will be created automatically. It is not exactly overwrite

Oh, sorry, I missed the Assert.fail.

Yet still, not sure that the golden data should be automatically created in this case.
IMO, better have one single mechanism to overwrite golden data (e.g., when special flag is set), and in case when there is no file with golden data just output that file was not found.

eupp · 2025-02-18T17:44:00Z

...org/jetbrains/kotlinx/lincheck_test/integration/KotlinImmutableCollectionsIntegrationTest.kt

+
+    @Test
+    fun `tests contract list GuavaImmutableListTest_list`() {
+//        runGradleTest(


Please use @Ignore instead and TODO comment explaining why the test is disabled for now.

eupp · 2025-02-18T17:44:24Z

src/jvm/test/org/jetbrains/kotlinx/lincheck_test/integration/AbstractIntegrationTest.kt

+import java.io.FileOutputStream
+import java.util.*
+
+abstract class AbstractIntegrationTest {


Suggested change

abstract class AbstractIntegrationTest {

abstract class AbstractTraceDebuggerIntegrationTest {

eupp · 2025-02-18T17:45:03Z

...org/jetbrains/kotlinx/lincheck_test/integration/KotlinImmutableCollectionsIntegrationTest.kt

+
+import org.junit.Test
+
+class KotlinImmutableCollectionsIntegrationTest: AbstractIntegrationTest() {


Suggested change

class KotlinImmutableCollectionsIntegrationTest: AbstractIntegrationTest() {

class KotlinImmutableCollectionsTraceDebuggerIntegrationTest: AbstractTraceDebuggerIntegrationTest() {

eupp · 2025-02-18T17:45:42Z

src/jvm/test/org/jetbrains/kotlinx/lincheck_test/integration/AbstractIntegrationTest.kt

+ * with this file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+package org.jetbrains.kotlinx.lincheck_test.integration


Also please move the package to trace_debugger.integration.

ivandev0 · 2025-02-18T18:26:59Z

Thanks for the review! I will fix everything on Monday.

ivandev0 · 2025-02-24T19:09:13Z

All changes are in fixup commits.

eupp · 2025-02-27T19:17:31Z

Also added @ndkoval to have a final look before we merge.

ndkoval · 2025-02-28T12:40:37Z

buildSrc/src/main/kotlin/TraceDebuggerTasks.kt

@@ -30,6 +30,7 @@ fun Project.registerTraceDebuggerTasks() {

    val traceDebuggerFatJar = tasks.register<Jar>("traceDebuggerFatJar") {
        archiveBaseName.set("fat-trace-debugger")
+        archiveVersion.set("SNAPSHOT")


should it always be SNAPSHOT? Can we use the default version instead?

What do you mean by "default version"? If we leave it as is, then the version will be different for each release. It is hard to work with that because we should manually update this version in the plugin.

You can obtain the current version of Lincheck from the code, see

lincheck/src/jvm/main/org/jetbrains/kotlinx/lincheck/LinChecker.kt

Line 339 in f2011d4

internal val lincheckVersion by lazy {

Yes, but it will not work for the plugin and the plugin has exactly the same logic

It's fine to change the jar name but please do not change the version.

This one is our internal fat jar. It is not published anywhere (yet). We can push it as is and I will create a task to think about how to extract lincheck version for the plugin.

You don't need to extract anything, you can just change the JAR name to "lincheck-fat-jar.jar"

Please note that it should not be fat-trace-debugger, this is just Lincheck

buildSrc/src/main/kotlin/TraceDebuggerIntegrationTestsTasks.kt

ndkoval · 2025-02-28T12:53:55Z

...est/org/jetbrains/kotlinx/trace_debugger/integration/AbstractTraceDebuggerIntegrationTest.kt

+    protected fun runGradleTest(
+        testClassName: String,
+        testMethodName: String,
+        vararg gradleCommand: String,


Can gradleCommand vary or should it always be jvmTest/test?

Let's also do not use vararg here, as it makes the test code less intuitive

Can gradleCommand vary or should it always be jvmTest/test?

Yes, it can change and it can depend on the project's infra that we are testing

Let's also do not use vararg here, as it makes the test code less intuitive

Do you propose to use an array here? Or do you mean to pass it as comma-separated string and parse later? I would like to keep it flexible to allow running multiple commands

Yeah, if you think this is better, no problem.

ndkoval · 2025-02-28T12:54:45Z

...kotlinx/trace_debugger/integration/KotlinImmutableCollectionsTraceDebuggerIntegrationTest.kt

+    @Ignore
+    @Test
+    fun `tests contract list GuavaImmutableListTest_list`() {
+        runGradleTest(


please always add argument names

ndkoval · 2025-02-28T12:56:44Z

...est/org/jetbrains/kotlinx/trace_debugger/integration/AbstractTraceDebuggerIntegrationTest.kt

+        }
+
+        // TODO decide how to test: with gold data or run twice?
+        val goldDataFile = File(getGolderDataPathFor(testClassName, testMethodName))


In Lincheck, we use the "expected output" term. Please use it here as well for consistency.

ndkoval · 2025-02-28T12:58:30Z

...kotlinx/trace_debugger/integration/KotlinImmutableCollectionsTraceDebuggerIntegrationTest.kt

+
+    @Ignore
+    @Test
+    fun `tests contract list GuavaImmutableListTest_list`() {


where can I find the expected output file?

Will add an example

ndkoval · 2025-02-28T13:00:44Z

...est/org/jetbrains/kotlinx/trace_debugger/integration/AbstractTraceDebuggerIntegrationTest.kt

+import java.io.FileOutputStream
+import java.util.*
+
+abstract class AbstractTraceDebuggerIntegrationTest {


Let's run these tests only in the trace debugger mode

We are running these tests in this mode, please check buildGradleInitScriptToDumpTrace

ndkoval · 2025-02-28T13:01:33Z

build.gradle.kts

    val jvmTest = named<Test>("jvmTest") {
+        dependsOn(traceDebuggerIntegrationTestsPrerequisites)


Let's create a separate source set and a separate task for integration tests for Trace Debugger, and perform all the related logic only in the trace debugger mode

Let's do it separately for every trace debugger test. I will create a ticket for that

The earlier version was "2.36-SNAPSHOT". What would mean that every time the new version of lincheck comes out, the corresponding path to fat jar must be changed in every project that uses trace debugger.

After the dump was published, we finished the execution because otherwise we stuck awaiting a plugin that is not necessarily present.

…ntegration tests

…s for integration tests

ndkoval · 2025-03-04T17:07:12Z

@ivandev0 can we please use the same human-readable output format as we use in other trace representation tests?

ivandev0 · 2025-03-04T17:20:45Z

@ivandev0 can we please use the same human-readable output format as we use in other trace representation tests?

Can you please point me where to look?

ndkoval · 2025-03-04T17:56:53Z

I guess you need the appendTrace function, but please take a look at any representation test as well, to check whether something else is necessary.

ndkoval added the Trace Debugger label Feb 10, 2025

ivandev0 force-pushed the kylchik/integration_tests branch from ea27b16 to 29c81c3 Compare February 10, 2025 16:22

ivandev0 requested a review from eupp February 10, 2025 18:16

ivandev0 marked this pull request as ready for review February 10, 2025 18:16

eupp requested changes Feb 18, 2025

View reviewed changes

ivandev0 force-pushed the kylchik/integration_tests branch from 29c81c3 to 3241b60 Compare February 24, 2025 19:08

ivandev0 requested a review from eupp February 24, 2025 19:30

eupp approved these changes Feb 26, 2025

View reviewed changes

ivandev0 force-pushed the kylchik/integration_tests branch from 3241b60 to 3b7b9b4 Compare February 27, 2025 11:23

eupp requested a review from ndkoval February 27, 2025 19:17

ndkoval requested changes Mar 3, 2025

View reviewed changes

ivandev0 added 10 commits March 3, 2025 13:46

Show more user-friendly error message if the test method is not public

29f2194

Use TraceDebuggerAgent arguments to parse class and method names

f36290f

Use just SNAPSHOT version when building fat jat for trace debugger

293c5bc

The earlier version was "2.36-SNAPSHOT". What would mean that every time the new version of lincheck comes out, the corresponding path to fat jar must be changed in every project that uses trace debugger.

Allow writing execution trace into the file if requested

3ce3a82

After the dump was published, we finished the execution because otherwise we stuck awaiting a plugin that is not necessarily present.

Create an abstract test class to run integration tests

3ab52e0

fixup! Create an abstract test class to run integration tests

d8a7353

Implement infrastructure to download all the necessary projects for i…

8921115

…ntegration tests

fixup! Implement infrastructure to download all the necessary project…

e183be7

…s for integration tests

Add an example of what the integration test should look like

62be602

fixup! Add an example of what the integration test should look like

ad95102

ivandev0 force-pushed the kylchik/integration_tests branch from 3b7b9b4 to ad95102 Compare March 3, 2025 12:46

ivandev0 requested a review from ndkoval March 3, 2025 12:47


		val isInTraceDebuggerMode = System.getProperty("lincheck.traceDebuggerMode", "false").toBoolean()
		private const val traceDebuggerModeProperty = "lincheck.traceDebuggerMode"

	abstract class AbstractIntegrationTest {
	abstract class AbstractTraceDebuggerIntegrationTest {


		import org.junit.Test

		class KotlinImmutableCollectionsIntegrationTest: AbstractIntegrationTest() {

	class KotlinImmutableCollectionsIntegrationTest: AbstractIntegrationTest() {
	class KotlinImmutableCollectionsTraceDebuggerIntegrationTest: AbstractTraceDebuggerIntegrationTest() {

		val jvmTest = named<Test>("jvmTest") {
		dependsOn(traceDebuggerIntegrationTestsPrerequisites)

Implement infrastructure to check recorded trace #510

Are you sure you want to change the base?

Implement infrastructure to check recorded trace #510

Conversation

ivandev0 commented Feb 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivandev0 commented Feb 18, 2025

ivandev0 commented Feb 24, 2025

eupp commented Feb 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ndkoval Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ndkoval commented Mar 4, 2025

ivandev0 commented Mar 4, 2025

ndkoval commented Mar 4, 2025

ndkoval Mar 4, 2025 •

edited

Loading