Skip to content

Add overdue tests for FakeNetwork - #9638

Merged
swankjesse merged 2 commits into
mainfrom
jwilson.0807.network_tests
Aug 8, 2026
Merged

Add overdue tests for FakeNetwork#9638
swankjesse merged 2 commits into
mainfrom
jwilson.0807.network_tests

Conversation

@swankjesse

Copy link
Copy Markdown
Collaborator

The test that uses real OkHttp is good to confirm it works, but it isn't useful for debugging problems with FakeNetwork.

The test that uses real OkHttp is good to confirm it works, but it
isn't useful for debugging problems with FakeNetwork.
@JvmField
val clientTestRule = OkHttpClientTestRule()

val taskRunner = TaskRunner.INSTANCE

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's good you can use either, but would you typically use TaskFaker to be 100% deterministic?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

YES! I need to make TaskFaker able to fake out wait + notify.

@swankjesse
swankjesse force-pushed the jwilson.0807.network_tests branch from 66a4756 to 6c187a5 Compare August 7, 2026 23:48
}
}

internal class SocketSource(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

FYI the new tests showed that there’s a race closing sockets which caused some tests to flake.

The fix was straightforward-enough . . . the source & sink offered by FakeSocket needs to track its own closed state. Previously I was relying on Socket.cancel() to do that, but it closes the peer stream asynchronously and we don’t want that.

@swankjesse
swankjesse merged commit d1e57f3 into main Aug 8, 2026
23 of 26 checks passed
@swankjesse
swankjesse deleted the jwilson.0807.network_tests branch August 8, 2026 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants