search

Kotlin / kotlinx.coroutines

Library support for Kotlin coroutines
Apache License 2.0
12.77k stars 1.82k forks source link

Fix documentation about the default timeout for runTest #4099

Closed OliverO2 closed 1 month ago

OliverO2 commented 1 month ago

The default timeout for runTest changed from 10 seconds to 60 seconds in 9a98eabed91c71cb2da9b0fe061139632e08990b. This PR makes the README reflect that change.

dkhalanskyjb commented 1 month ago

This PR makes the README reflect that change

Good catch, thanks!

includes documentation for the added system property

The idea was that the system property should very rarely be needed. If that's true, mentioning it in the runTest documentation should be enough. Do you think the need is widespread enough to be mentioned in the introductory overview that is the README?

OliverO2 commented 1 month ago

I'd also assume that the property would be used in rare cases (slow CI, many similar runTest invocations, JVM only). I don't mind if we'd waive mentioning it in the README.

Possible downside: If people run into timeout problems they would most probably consider adding the timeout parameter (hinted at by the README and assisted by the IDE). Will they read the runTest documentation on that occasion?

Having said that, it's no big deal for me: If you'd prefer, I can just drop that section on the property.

dkhalanskyjb commented 1 month ago

Will they read the runTest documentation on that occasion?

Fair, but will they remember to reread the README that they skimmed when they initially learned about the coroutines test framework?

Having said that, it's no big deal for me: If you'd prefer, I can just drop that section on the property.

Yes, I think it's better to keep the README approachable and free of technical details.

dkhalanskyjb commented 1 month ago

Thanks!

OliverO2 commented 1 month ago

My pleasure!