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

Fix a test error in gson/extra #936

Closed
wants to merge 1 commit into from
Closed

Fix a test error in gson/extra #936

wants to merge 1 commit into from

Conversation

thaibui
Copy link

@thaibui thaibui commented Sep 28, 2016

GraphAdapterBuilderTest fails on master when execute mvn -e clean test under json/extras directory; this PR should fix the issue.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@thaibui
Copy link
Author

thaibui commented Sep 28, 2016

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@swankjesse
Copy link
Collaborator

This test case used to pass, but it was regressed with this change:
8d5a413

@inder123 thoughts on self-references?

@Marcono1234
Copy link
Collaborator

Probably obsolete now due to #1959 (comment).

Though at least personally I think it might be worth revisiting the self-reference detection of ReflectiveTypeAdapterFactory again at some point. It only detects direct self-references so this only helps for a very specific case. Additionally, if this was only done to support serialization of Throwable, then removing the direct self-reference check would not be so problematic because you should not serialize JDK classes using reflection.

@eamonnmcmanus
Copy link
Member

Closing as obsolete. If we do want to revisit the self-reference detection of ReflectiveTypeAdapterFactory then that should be a separate issue.

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.

None yet

5 participants