Skip to content

Boolean getter support added (Issue #138)#136

Closed
bvrakvs wants to merge 9 commits intohamcrest:masterfrom
bvrakvs:master
Closed

Boolean getter support added (Issue #138)#136
bvrakvs wants to merge 9 commits intohamcrest:masterfrom
bvrakvs:master

Conversation

@bvrakvs
Copy link
Copy Markdown

@bvrakvs bvrakvs commented Feb 25, 2016

This request is to support the is prefix for Boolean object getters. Since PropertyDescriptor that is created lacks readMethod if the getter does not start with get. For example, classes that are generated by JAXB, always contain getters for Boolean types with is prefix, and without this minor fix, I cannot match those fields in any of my UT cases.

@bvrakvs
Copy link
Copy Markdown
Author

bvrakvs commented Feb 26, 2016

Will there be any response to this pull request? Is there any possibility for this fix to be included in hamcrest:master ? I do not understand why Boolean getters with is prefix are being ignored?

@mikulucky
Copy link
Copy Markdown

@bureaquete This is probably not being included because your PR has failed the Continuous Integration

@bvrakvs
Copy link
Copy Markdown
Author

bvrakvs commented Feb 26, 2016

@mikulucky why though? Everything checks out in my local, no test failure or anything. Cannot find out the reason of CI failure.

@mikulucky
Copy link
Copy Markdown

@bureaquete the test failures seem to be with your changes on OpenJDK7 https://travis-ci.org/hamcrest/JavaHamcrest/builds/111651291

@bvrakvs
Copy link
Copy Markdown
Author

bvrakvs commented Feb 26, 2016

@mikulucky I haven't changed anything but the file in my commits. If you check last couple pull requests, all have the same failure, but from console I cannot figure out the exact reason.

@zaphod42
Copy link
Copy Markdown
Member

It looks like the build is hitting a problem in Travis combined with a problem in the JDK. See travis-ci/travis-ci#5227

This isn't anything to do with these specific changes.

@bvrakvs
Copy link
Copy Markdown
Author

bvrakvs commented Mar 3, 2016

@zaphod42 I tried to use the similar fix from your link, but did not resolve the build failure. Weird issue indeed...

@bvrakvs bvrakvs changed the title Boolean getter support added Boolean getter support added (#138) Mar 22, 2016
@bvrakvs bvrakvs changed the title Boolean getter support added (#138) Boolean getter support added (Issue #138) Mar 22, 2016
@sf105
Copy link
Copy Markdown
Member

sf105 commented Apr 6, 2016

I've just added a test for what I think this problem is and it works on my machine without any code changes (Java 7). Please submit a failing test.

@sf105 sf105 closed this Apr 6, 2016
@sf105
Copy link
Copy Markdown
Member

sf105 commented Apr 8, 2016

The fundamental problem appears to be that the JAXB people didn't read the JavaBean spec. That said, I just tried a version of your code and couldn't get it to work because there's no property descriptor for is. Please add a test to demonstrate the behaviour.

@sf105 sf105 reopened this Apr 8, 2016
@bvrakvs
Copy link
Copy Markdown
Author

bvrakvs commented Apr 11, 2016

@sf105 I couldn't reproduce the issue I have been facing... In my code, somehow I can get a PropertyDescriptor for the Boolean field with an empty getter method, and my first modification was to fix that issue. I tried to use exact same class from my project, but still got null PropertyDescriptor. Not sure what causes this different behaviour.

Regardless I have added an extra code in PropertyUtil to cover this empty case.

@nhojpatrick
Copy link
Copy Markdown
Member

@buraequete fancy rebasing this from master, as hamcrest-core and hamcrest-library have been refactored a lot and also deprecated, so that everything is just in hamcrest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Closed

Development

Successfully merging this pull request may close these issues.

5 participants