configure to run tests in parallel and eliminate global state #73

christophsturm · 2022-10-04T12:59:56Z

this speeds up a ./gradlew cleanTest test run from 14 seconds to 5 seconds on my m1 macbook pro.
If you like I can also create a PR that converts the test suite to https://github.com/failgood/failgood, which will probably make it even faster, simpler and nicer to read </commercial>

TWiStErRob

Few thoughts from a watcher (not maintainer).

kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/DebugFunctionTest.kt

TWiStErRob · 2022-10-04T20:49:05Z

kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/DebugFunctionTest.kt

+      if (this is RuntimeException && message != null && message!!.startsWith("result:"))
+        return message!!.substringAfter("result:")


could this be a typed custom exception instead?

class DebugFunctionFailure(result: String) : RuntimeException(result)

+

if (ex.cause is DebugFunctionFailure) return ex.cause.message

i wanted to keep it simple because the exception would need to be declared in the main method and in the method catching it.

Ah, that sounds like there may be a fun classloader issue there. Curious what @bnorm thinks.

TWiStErRob · 2022-10-04T20:53:33Z

kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/MultiParameterFunctionTest.kt

+val cnt = AtomicLong()
+fun nextMain() = "${cnt.addAndGet(1)}"


To remove global state fully, "main${UUID.randomUUID()}.kt" might be a more decoupled way. or even using the temp functionality of JUnit which has an added benefit of deleting the file which makes sure that the file handles are closed.

thats an interesting thought, using a counter is also some kind of shared state, I never thought about it that way. I usually always use UUIDs but this time i wanted to avoid too long indentifiers.

Note that Gradle has parallelization too for tests and if that's enabled there'll be multiple JVMs launched for each test task, so in memory synchronized state is not enough.

Check out Test.maxParallelForks property (example usage)

sure but multiple vms is slow so thats not what I'm targetting here.

Depends on the setup, I would spawn multiple VMs because the compilation is the bottleneck in these tests. Although that might be parallelized anyway with the current setup you did.

About targeting this or not: fair, I just wanted to highlight that "global" state is not fully removed, because the disk is still shared (same folder). And you got that.

I'll try a measurement on the weekend, I'm curious how this change or using forks changes the count to validate if JVMs are really as expensive. (I have 28 cores and 96GB) Don't need to wait for me on anything, just FYI.

btw if you have so many cores, can you do me a favor and run the test suite of my orm library https://github.com/christophsturm/the.orm and tell me how fast it runs?

kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/InfixFunctionTest.kt

kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/compiler.kt

kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/MultiParameterFunctionTest.kt

christophsturm · 2022-10-05T08:48:51Z

@TWiStErRob thanks for your feedback, I have found out that the whole "making file name unique" effort was not needed and that the only problem was System.out. so now this PR turned out really simple.

TWiStErRob · 2022-10-05T08:50:14Z

Niiice! (You can resolve most comments then)

christophsturm · 2022-10-05T08:52:52Z

about the formatting, maybe we could also add the kotlinter gradle plugin to run formatting locally and not only check it on CI

bnorm

First off, I apologize for not being very active in this repository recently. Life has kept me busy elsewhere but I'm hoping to dedicate some more time to this repo again soon.

Secondly, I appreciate the contribution and am excited about the reduction in test times! Thank you!

However, I'm not sure I like the conversion to exceptions for these tests. Use of println was sort of a hack to emulate a proper logging framework. I wonder if we can add a top-level StringBuilder to the dbg code which can be accessed after main is executed or if we can integrate with JUL somehow.

Regardless, I don't think this change reduces the test coverage at all so I'm going to merge it as-is and consider alternatives at a later time.

christophsturm · 2023-03-01T19:04:15Z

However, I'm not sure I like the conversion to exceptions for these tests. Use of println was sort of a hack to emulate a proper logging framework. I wonder if we can add a top-level StringBuilder to the dbg code which can be accessed after main is executed or if we can integrate with JUL somehow.

yes, I just fixed it in the simplest way that I could think of, the idea of the PR was mainly to show what problems we have when running test in parallel.

also I'm excited to see activity again in this repo!

bump junit, run tests in parallel.

fd6551a

TWiStErRob reviewed Oct 4, 2022

View reviewed changes

TWiStErRob reviewed Oct 5, 2022

View reviewed changes

kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/compiler.kt Outdated Show resolved Hide resolved

kotlin-power-assert-plugin/src/test/kotlin/com/bnorm/power/MultiParameterFunctionTest.kt Outdated Show resolved Hide resolved

fix global state by not using System.out

811640f

christophsturm force-pushed the junit-parallel branch from 264bc8d to 811640f Compare October 5, 2022 08:44

bnorm approved these changes Mar 1, 2023

View reviewed changes

bnorm merged commit be90be5 into bnorm:master Mar 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

configure to run tests in parallel and eliminate global state #73

configure to run tests in parallel and eliminate global state #73

christophsturm commented Oct 4, 2022 •

edited

Loading

TWiStErRob left a comment •

edited

Loading

TWiStErRob Oct 4, 2022 •

edited

Loading

christophsturm Oct 4, 2022

TWiStErRob Oct 5, 2022

TWiStErRob Oct 4, 2022

christophsturm Oct 4, 2022

TWiStErRob Oct 5, 2022 •

edited

Loading

christophsturm Oct 5, 2022

TWiStErRob Oct 5, 2022

christophsturm Oct 5, 2022

christophsturm commented Oct 5, 2022

TWiStErRob commented Oct 5, 2022 •

edited

Loading

christophsturm commented Oct 5, 2022

bnorm left a comment

christophsturm commented Mar 1, 2023

		if (this is RuntimeException && message != null && message!!.startsWith("result:"))
		return message!!.substringAfter("result:")

		val cnt = AtomicLong()
		fun nextMain() = "${cnt.addAndGet(1)}"

configure to run tests in parallel and eliminate global state #73

configure to run tests in parallel and eliminate global state #73

Conversation

christophsturm commented Oct 4, 2022 • edited Loading

TWiStErRob left a comment • edited Loading

Choose a reason for hiding this comment

TWiStErRob Oct 4, 2022 • edited Loading

Choose a reason for hiding this comment

christophsturm Oct 4, 2022

Choose a reason for hiding this comment

TWiStErRob Oct 5, 2022

Choose a reason for hiding this comment

TWiStErRob Oct 4, 2022

Choose a reason for hiding this comment

christophsturm Oct 4, 2022

Choose a reason for hiding this comment

TWiStErRob Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

christophsturm Oct 5, 2022

Choose a reason for hiding this comment

TWiStErRob Oct 5, 2022

Choose a reason for hiding this comment

christophsturm Oct 5, 2022

Choose a reason for hiding this comment

christophsturm commented Oct 5, 2022

TWiStErRob commented Oct 5, 2022 • edited Loading

christophsturm commented Oct 5, 2022

bnorm left a comment

Choose a reason for hiding this comment

christophsturm commented Mar 1, 2023

christophsturm commented Oct 4, 2022 •

edited

Loading

TWiStErRob left a comment •

edited

Loading

TWiStErRob Oct 4, 2022 •

edited

Loading

TWiStErRob Oct 5, 2022 •

edited

Loading

TWiStErRob commented Oct 5, 2022 •

edited

Loading