Skip to content

Make Dispatchers.setMain affect all delays in tests. #3049

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.

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

Closed
wants to merge 7 commits into from

Conversation

dkhalanskyjb
Copy link
Collaborator

@dkhalanskyjb dkhalanskyjb commented Nov 25, 2021

Option 2.

qwwdfsad and others added 2 commits November 24, 2021 15:54
@dkhalanskyjb dkhalanskyjb marked this pull request as ready for review November 25, 2021 09:13
Copy link
Collaborator

@qwwdfsad qwwdfsad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit concerned this is way too intrusive change with potentially non-trivial interactions with existing/running code (e.g. now the source of default delay can be changed on the fly).

If you do not have objections, I would prefer sticking with option 1 for the initial release

@dkhalanskyjb
Copy link
Collaborator Author

I would also prefer the first option initially, at least until we deal with the initial feedback on both the updated test framework and the change to the default delay. Otherwise, the changes could just be too overwhelming: it's not easy now to diagnose the situation where the default delay is not used for some reason, and the consequences of entering such a situation would become much more significant.

That said, I'd argued that, in code that's heavily using Dispatchers.Main, the problem of non-trivial interactions is already just as pronounced, as the only place where the default delay could change is in setMain.

Base automatically changed from fix-main-delay to develop December 10, 2021 08:23
@dkhalanskyjb
Copy link
Collaborator Author

#3094 contains some examples of when delay-skipping is undesirable in tests.

@1zaman
Copy link
Contributor

1zaman commented Feb 5, 2022

This might cause similar issues as in #3173, with actual test infrastructure delays being controlled by the test dispatcher. Perhaps this can be addressed by using another delay-handling dispatcher to run delays in the test infrastructure, as planned by the Turbine library.

Note that as described in #3173, Dispatchers.setMain already affects all delays in Robolectric tests, which causes inconsistencies with non-Robolectric tests.

@qwwdfsad
Copy link
Collaborator

This PR can be now closed, can't it?

@dkhalanskyjb
Copy link
Collaborator Author

Sure.

@qwwdfsad qwwdfsad deleted the fix-main-delay-test-2 branch October 18, 2022 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants