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

Add unit tests for OnBackPressedDispatcher #1648

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Add unit tests for OnBackPressedDispatcher #1648

merged 1 commit into from
Oct 31, 2023

Conversation

swankjesse
Copy link
Collaborator

No description provided.

@swankjesse
Copy link
Collaborator Author

⚠️ stacked PR!

@swankjesse
Copy link
Collaborator Author

Part of #1458

JakeWharton
JakeWharton approved these changes Oct 31, 2023
View reviewed changes
Copy link
Collaborator

@JakeWharton JakeWharton left a comment

Choose a reason for hiding this comment

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

Are we testing real codepaths here? There's a lot of fake infrastructure in play and it's hard to understand what real behavior we're trying to test.

Base automatically changed from jwilson.1027.ZiplineTreehouseApi.Host to trunk October 31, 2023 19:22
@swankjesse
Copy link
Collaborator Author

The TreehouseAppContent is the real codepath this is exercising. It’s at the center of 3 different lifecycles (view attach, code load, content), and so it’s been very scary to change. With this I’m hoping to make it a little bit better.

I also see some options to promote other things to be under test, perhaps by moving the boundaries of what’s an interface and what’s not.

@swankjesse swankjesse force-pushed the jwilson.1027.back_listener_cleared branch from e3659c2 to 4d8dd7e Compare October 31, 2023 19:29
@swankjesse swankjesse enabled auto-merge (squash) October 31, 2023 19:29
@JakeWharton
Copy link
Collaborator

It's probably easier to see in the IDE where I can click around more easily. It's hard to see exact boundaries in the diff.

@swankjesse swankjesse merged commit d51f1a6 into trunk Oct 31, 2023
8 checks passed
@swankjesse swankjesse deleted the jwilson.1027.back_listener_cleared branch October 31, 2023 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veyndan veyndan veyndan approved these changes

@JakeWharton JakeWharton JakeWharton approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@swankjesse @JakeWharton @veyndan @squarejesse