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

Add TypeMatcher in xmlunit-matchers #137

Merged
merged 6 commits into from Aug 24, 2018

Conversation

Projects
None yet
4 participants
@krystiankaluzny
Member

krystiankaluzny commented Aug 22, 2018

TypeMatcher allows to check if an examined string value can be converted to specified type and perform matching on the converted value.
For now 4 types are support: BigDecimal, Double, Integer and Boolean
This feature is response on #133 and works well with EvaluateXPathMatcher:

assertThat(xml, hasXPath("count(//fruits/fruit)", asBigDecimal(is(greaterThan(BigDecimal.ONE)))));

@krystiankaluzny krystiankaluzny requested a review from bodewig Aug 22, 2018

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Aug 22, 2018

Coverage Status

Coverage increased (+0.05%) to 91.982% when pulling a2e4e59 on type-matcher into 3fda805 on master.

coveralls commented Aug 22, 2018

Coverage Status

Coverage increased (+0.05%) to 91.982% when pulling a2e4e59 on type-matcher into 3fda805 on master.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Aug 22, 2018

Coverage Status

Coverage increased (+0.05%) to 91.982% when pulling e570015 on type-matcher into 3fda805 on master.

coveralls commented Aug 22, 2018

Coverage Status

Coverage increased (+0.05%) to 91.982% when pulling e570015 on type-matcher into 3fda805 on master.

@bodewig

Many thanks, looks good.

I'm not totally sure about accepting 0/1 as booleans, one could argue that it should also accept on/off or something similar.

We may want to extend this with a TypeMatcher<Date> later.

@krystiankaluzny

This comment has been minimized.

Show comment
Hide comment
@krystiankaluzny

krystiankaluzny Aug 23, 2018

Member

Using 1/0 as true/false is natural in C/C++ or, at least it was, when I used those languages few years ago.
So it was natural for me to treat 0/1 as boleans.
If you think it is confusing, we can remove it now and if users need this feature, we will simply add it.

Member

krystiankaluzny commented Aug 23, 2018

Using 1/0 as true/false is natural in C/C++ or, at least it was, when I used those languages few years ago.
So it was natural for me to treat 0/1 as boleans.
If you think it is confusing, we can remove it now and if users need this feature, we will simply add it.

@phbenisc

This comment has been minimized.

Show comment
Hide comment
@phbenisc

phbenisc Aug 23, 2018

Contributor

You might also consider following information: In XML Schema 1.0 a boolean can be one of these: true, false, 1 or 0. See https://www.w3.org/TR/xmlschema-2/#boolean . In XML Schema 1.1 this Issue is "fixed" https://www.w3.org/TR/xmlschema11-2/#boolean

I also believe prior to SOAP 1.2 XML Schema 1.0 is used. Quite a few SOAP-Services should accept 1 and 0 as valid boolean values. So you could argue that it still is quite common. I personally think 1/0 is nowadays a bad practice and should be avoided.

Contributor

phbenisc commented Aug 23, 2018

You might also consider following information: In XML Schema 1.0 a boolean can be one of these: true, false, 1 or 0. See https://www.w3.org/TR/xmlschema-2/#boolean . In XML Schema 1.1 this Issue is "fixed" https://www.w3.org/TR/xmlschema11-2/#boolean

I also believe prior to SOAP 1.2 XML Schema 1.0 is used. Quite a few SOAP-Services should accept 1 and 0 as valid boolean values. So you could argue that it still is quite common. I personally think 1/0 is nowadays a bad practice and should be avoided.

@krystiankaluzny

This comment has been minimized.

Show comment
Hide comment
@krystiankaluzny

krystiankaluzny Aug 23, 2018

Member

Ok, to be consistent I also removed 0/1 to boolean conversion from assertj module

Member

krystiankaluzny commented Aug 23, 2018

Ok, to be consistent I also removed 0/1 to boolean conversion from assertj module

@bodewig

This comment has been minimized.

Show comment
Hide comment
@bodewig

bodewig Aug 24, 2018

Member

we may want to note this as a breaking change inside the AsserJ module.

Member

bodewig commented Aug 24, 2018

we may want to note this as a breaking change inside the AsserJ module.

@bodewig bodewig merged commit 412916b into master Aug 24, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.05%) to 91.982%
Details

bodewig added a commit that referenced this pull request Aug 24, 2018

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