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.

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

Don't fail when we can't find an AutoValue Builder setter method #111

Merged
merged 4 commits into from
Nov 19, 2019

Conversation

msridhar
Copy link
Collaborator

Before, we would fail with a RuntimeException if we couldn't match what we thought was an AutoValue property to a corresponding setter in the builder. But, we don't currently handle AutoValue extension (see #110), and adding full and proper handling will be pretty non-trivial. So, instead of failing for these cases, just tolerate them.

Add a test case using the AutoValue Parcel extension to expose the problem, along with some stub Android classes so we don't need to pull in a full Android dependence.

@msridhar msridhar requested a review from kelloggm November 16, 2019 23:48
Copy link
Owner

@kelloggm kelloggm left a comment

Choose a reason for hiding this comment

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

I would expect one of the new tests you added to fail due to the lack of support for extensions, but it doesn't look like any of them are skipped. Can you explain why?

Do we have a test that would only pass if we did support extensions? If so, can you add it with an @skip-test annotation at the top of the test?

@msridhar
Copy link
Collaborator Author

Sorry for the confusion. The test passes because this PR disables the check for a matching builder setter. The test fails on current master with a RuntimeException. I will update comments to be clearer

@msridhar msridhar merged commit dca6d91 into master Nov 19, 2019
@msridhar msridhar deleted the av-builder-missing-setter-dont-fail branch November 19, 2019 17:04
msridhar referenced this pull request in msridhar/object-construction-checker Nov 19, 2019
Before, we would fail with a `RuntimeException` if we couldn't match what we thought was an AutoValue property to a corresponding setter in the builder.  But, we don't currently handle AutoValue extensions (see #110), and adding full and proper handling will be pretty non-trivial.  So, instead of failing for these cases, just tolerate them.

Add a test case using the AutoValue Parcel extension to expose the problem, along with some stub Android classes so we don't need to pull in a full Android dependence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants