Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reactive streams TCK tests are too slow #938

Closed
fwbrasil opened this issue Dec 17, 2024 · 2 comments · Fixed by #939
Closed

Reactive streams TCK tests are too slow #938

fwbrasil opened this issue Dec 17, 2024 · 2 comments · Fixed by #939
Labels
tech debt

Comments

@fwbrasil
Copy link
Collaborator

A quick test suite is an important design principle of Kyo's codebase. The new tests introduced by #896 are taking much longer than all the other tests combined. We need to find a reliable way to execute them faster. cc/ @HollandDM
@HollandDM HollandDM mentioned this issue Dec 17, 2024
Merged
@HollandDM
Copy link
Contributor

Idk why reactive-streams-tck run so slow, in fs2 its also take about 30s for the suites.
we can move reactive-streams-tck and reactive-streams interrop out of kyo's main repo if we have too.
In the mean time, I'll try to figure things out.

@HollandDM
Copy link
Contributor

as far as i can tell, reactive-streams-tck use a lot of complicate computations, including the inefficient java concurrent one (i guess?), so it run slow. I created a dummy publisher which use only synchronous expressions and it still takes ~30s to complete

hearnadam pushed a commit that referenced this issue Dec 23, 2024
@HollandDM
[kyo-reactive-streams]: Remove un-necessary sleep time in test cases (#…
b7d6424 
…939)

Originally, `Async.sleep` was used to mimic the real-life delay of a
publisher/subscriber, but in the `reactive-streams-tck` test cases, it
provides little to no benefit while significantly slowing down the
overall test. Additionally, a quick look at `fs2` and `zio` shows that
they also do not use sleep in their test cases.

This PR takes it a step further by removing `Async.sleep` from all
`kyo-reactive-stream` test cases. Since this feature is primarily about
correctly combining effects, I believe that if individual components
have been thoroughly tested, we can proceed with just the minimal
testing.

Furthermore, because `StreamSubscriber` has two strategies, I initially
duplicated the tests for both. However, I think it suffices to randomly
choose one strategy for each test in the suite.

Overall, on my MacBook M3, the old tests took `1 minute and 37 seconds`
to run, while the new tests complete in just `34 seconds`.

Aim to resolve #938
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
tech debt
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[kyo-reactive-streams]: Remove un-necessary sleep time in test cases HollandDM/kyo
2 participants
@fwbrasil @HollandDM