Skip to content

Commit

Permalink
fix: min/max type matchers must not apply the limits when cascading #396
Browse files Browse the repository at this point in the history
  • Loading branch information
Ronald Holshausen committed Aug 8, 2021
1 parent a816fb8 commit 2a12a32
Show file tree
Hide file tree
Showing 14 changed files with 251 additions and 93 deletions.
105 changes: 61 additions & 44 deletions core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/MatcherExecutor.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ fun <M : Mismatch> domatch(
mismatchFn: MismatchFactory<M>
): List<M> {
val result = matchers.rules.map { matchingRule ->
domatch(matchingRule, path, expected, actual, mismatchFn)
domatch(matchingRule, path, expected, actual, mismatchFn, matchers.cascaded)
}

return if (matchers.ruleLogic == RuleLogic.AND) {
Expand All @@ -114,7 +114,8 @@ fun <M : Mismatch> domatch(
path: List<String>,
expected: Any?,
actual: Any?,
mismatchFn: MismatchFactory<M>
mismatchFn: MismatchFactory<M>,
cascaded: Boolean
): List<M> {
return when (matcher) {
is RegexMatcher -> matchRegex(matcher.regex, path, expected, actual, mismatchFn)
Expand All @@ -123,10 +124,10 @@ fun <M : Mismatch> domatch(
is DateMatcher -> matchDate(matcher.format, path, expected, actual, mismatchFn)
is TimeMatcher -> matchTime(matcher.format, path, expected, actual, mismatchFn)
is TimestampMatcher -> matchDateTime(matcher.format, path, expected, actual, mismatchFn)
is MinTypeMatcher -> matchMinType(matcher.min, path, expected, actual, mismatchFn)
is MaxTypeMatcher -> matchMaxType(matcher.max, path, expected, actual, mismatchFn)
is MinMaxTypeMatcher -> matchMinType(matcher.min, path, expected, actual, mismatchFn) +
matchMaxType(matcher.max, path, expected, actual, mismatchFn)
is MinTypeMatcher -> matchMinType(matcher.min, path, expected, actual, mismatchFn, cascaded)
is MaxTypeMatcher -> matchMaxType(matcher.max, path, expected, actual, mismatchFn, cascaded)
is MinMaxTypeMatcher -> matchMinType(matcher.min, path, expected, actual, mismatchFn, cascaded) +
matchMaxType(matcher.max, path, expected, actual, mismatchFn, cascaded)
is IncludeMatcher -> matchInclude(matcher.value, path, expected, actual, mismatchFn)
is NullMatcher -> matchNull(path, actual, mismatchFn)
is ContentTypeMatcher -> matchContentType(path, ContentType.fromString(matcher.contentType), actual, mismatchFn)
Expand Down Expand Up @@ -381,29 +382,37 @@ fun <M : Mismatch> matchMinType(
path: List<String>,
expected: Any?,
actual: Any?,
mismatchFactory: MismatchFactory<M>
mismatchFactory: MismatchFactory<M>,
cascaded: Boolean
): List<M> {
logger.debug { "comparing ${valueOf(actual)} with minimum $min at $path" }
return if (actual is List<*>) {
if (actual.size < min) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have minimum $min", path))
} else {
emptyList()
}
} else if (actual is JsonValue.Array) {
if (actual.size < min) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have minimum $min", path))
} else {
emptyList()
}
} else if (actual is Element) {
if (actual.childNodes.length < min) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have minimum $min", path))
} else {
emptyList()
return if (!cascaded) {
when (actual) {
is List<*> -> {
if (actual.size < min) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have minimum $min", path))
} else {
emptyList()
}
}
is JsonValue.Array -> {
if (actual.size < min) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have minimum $min", path))
} else {
emptyList()
}
}
is Element -> {
if (actual.childNodes.length < min) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have minimum $min", path))
} else {
emptyList()
}
}
else -> matchType(path, expected, actual, mismatchFactory)
}
} else {
matchType(path, expected, actual, mismatchFactory)
matchType(path, expected, actual, mismatchFactory)
}
}

Expand All @@ -412,29 +421,37 @@ fun <M : Mismatch> matchMaxType(
path: List<String>,
expected: Any?,
actual: Any?,
mismatchFactory: MismatchFactory<M>
mismatchFactory: MismatchFactory<M>,
cascaded: Boolean
): List<M> {
logger.debug { "comparing ${valueOf(actual)} with maximum $max at $path" }
return if (actual is List<*>) {
if (actual.size > max) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have maximum $max", path))
} else {
emptyList()
}
} else if (actual is JsonValue.Array) {
if (actual.size > max) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have maximum $max", path))
} else {
emptyList()
}
} else if (actual is Element) {
if (actual.childNodes.length > max) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have maximum $max", path))
} else {
emptyList()
return if (!cascaded) {
when (actual) {
is List<*> -> {
if (actual.size > max) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have maximum $max", path))
} else {
emptyList()
}
}
is JsonValue.Array -> {
if (actual.size > max) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have maximum $max", path))
} else {
emptyList()
}
}
is Element -> {
if (actual.childNodes.length > max) {
listOf(mismatchFactory.create(expected, actual, "Expected ${valueOf(actual)} to have maximum $max", path))
} else {
emptyList()
}
}
else -> matchType(path, expected, actual, mismatchFactory)
}
} else {
matchType(path, expected, actual, mismatchFactory)
matchType(path, expected, actual, mismatchFactory)
}
}

Expand Down
25 changes: 13 additions & 12 deletions core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/Matchers.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ object Matchers : KLogging() {
category: String,
items: List<String>,
pathComparator: Comparator<String>
) = if (category == "body")
matchers.rulesForCategory(category).filter(Predicate {
matchesPath(it, items) > 0
})
else if (category == "header" || category == "query" || category == "metadata")
matchers.rulesForCategory(category).filter(Predicate { key ->
items.all { pathComparator.compare(key, it) == 0 }
})
else matchers.rulesForCategory(category)
) = when (category) {
"body" -> matchers.rulesForCategory(category).filter(Predicate {
matchesPath(it, items) > 0
})
"header", "query", "metadata" -> matchers.rulesForCategory(category).filter(Predicate { key ->
items.all { pathComparator.compare(key, it) == 0 }
})
else -> matchers.rulesForCategory(category)
}

@JvmStatic
@JvmOverloads
Expand Down Expand Up @@ -117,8 +117,8 @@ object Matchers : KLogging() {
pathComparator: Comparator<String> = Comparator.naturalOrder()
): MatchingRuleGroup {
val matcherCategory = resolveMatchers(matchers, category, path, pathComparator)
return if (category == "body")
matcherCategory.maxBy(Comparator { a, b ->
return if (category == "body") {
val result = matcherCategory.maxBy(Comparator { a, b ->
val weightA = calculatePathWeight(a, path)
val weightB = calculatePathWeight(b, path)
when {
Expand All @@ -131,7 +131,8 @@ object Matchers : KLogging() {
else -> -1
}
})
else {
result?.second?.copy(cascaded = parsePath(result.first).size != path.size) ?: MatchingRuleGroup()
} else {
matcherCategory.matchingRules.values.first()
}
}
Expand Down
16 changes: 11 additions & 5 deletions core/matchers/src/main/kotlin/au/com/dius/pact/core/matchers/Matching.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ object Matching : KLogging() {
matchers ?: MatchingRulesImpl())
}.filterNotNull())
} else {
list + HeaderMatchResult(values.key, listOf(HeaderMismatch(values.key, values.value.joinToString(separator = ", "), "",
list + HeaderMatchResult(values.key,
listOf(HeaderMismatch(values.key, values.value.joinToString(separator = ", "), "",
"Expected a header '${values.key}' but was missing")))
}
}
Expand Down Expand Up @@ -77,8 +78,11 @@ object Matching : KLogging() {
}
}
} else {
if (expected.body.isMissing() || expected.body.isNull() || expected.body.isEmpty()) BodyMatchResult(null, emptyList())
else BodyMatchResult(BodyTypeMismatch(expectedContentType.getBaseType(), actualContentType.getBaseType()), emptyList())
if (expected.body.isMissing() || expected.body.isNull() || expected.body.isEmpty())
BodyMatchResult(null, emptyList())
else
BodyMatchResult(BodyTypeMismatch(expectedContentType.getBaseType(), actualContentType.getBaseType()),
emptyList())
}
}

Expand Down Expand Up @@ -115,11 +119,13 @@ object Matching : KLogging() {
null -> acc + QueryMatchResult(entry.key, listOf(QueryMismatch(entry.key, entry.value.joinToString(","), "",
"Expected query parameter '${entry.key}' but was missing",
listOf("$", "query", entry.key).joinToString("."))))
else -> acc + QueryMatchResult(entry.key, QueryMatcher.compareQuery(entry.key, entry.value, value, expected.matchingRules))
else -> acc +
QueryMatchResult(entry.key, QueryMatcher.compareQuery(entry.key, entry.value, value, expected.matchingRules))
}
} + actual.query.entries.fold(emptyList<QueryMatchResult>()) { acc, entry ->
when (expected.query[entry.key]) {
null -> acc + QueryMatchResult(entry.key, listOf(QueryMismatch(entry.key, "", entry.value.joinToString(","),
null -> acc +
QueryMatchResult(entry.key, listOf(QueryMismatch(entry.key, "", entry.value.joinToString(","),
"Unexpected query parameter '${entry.key}' received",
listOf("$", "query", entry.key).joinToString("."))))
else -> acc
Expand Down
47 changes: 26 additions & 21 deletions core/matchers/src/test/groovy/au/com/dius/pact/core/matchers/MatcherExecutorSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ class MatcherExecutorSpec extends Specification {
@Unroll
def 'equals matcher matches using equals'() {
expect:
MatcherExecutorKt.domatch(EqualsMatcher.INSTANCE, path, expected, actual, mismatchFactory).empty == mustBeEmpty
MatcherExecutorKt.domatch(EqualsMatcher.INSTANCE, path, expected, actual, mismatchFactory, false).empty ==
mustBeEmpty

where:
expected | actual || mustBeEmpty
Expand Down Expand Up @@ -71,7 +72,8 @@ class MatcherExecutorSpec extends Specification {
@Unroll
def 'regex matcher matches using the provided regex'() {
expect:
MatcherExecutorKt.domatch(new RegexMatcher(regex), path, expected, actual, mismatchFactory).empty == mustBeEmpty
MatcherExecutorKt.domatch(new RegexMatcher(regex), path, expected, actual, mismatchFactory, false).empty ==
mustBeEmpty

where:
expected | actual | regex || mustBeEmpty
Expand All @@ -87,7 +89,8 @@ class MatcherExecutorSpec extends Specification {
@Unroll
def 'type matcher matches on types'() {
expect:
MatcherExecutorKt.domatch(TypeMatcher.INSTANCE, path, expected, actual, mismatchFactory).empty == mustBeEmpty
MatcherExecutorKt.domatch(TypeMatcher.INSTANCE, path, expected, actual, mismatchFactory, false).empty ==
mustBeEmpty

where:
expected | actual || mustBeEmpty
Expand Down Expand Up @@ -116,8 +119,8 @@ class MatcherExecutorSpec extends Specification {
@Unroll
def 'number type matcher matches on types'() {
expect:
MatcherExecutorKt.domatch(new NumberTypeMatcher(numberType), path, expected, actual, mismatchFactory).empty ==
mustBeEmpty
MatcherExecutorKt.domatch(new NumberTypeMatcher(numberType), path, expected, actual, mismatchFactory,
false).empty == mustBeEmpty

where:
numberType | expected | actual || mustBeEmpty
Expand Down Expand Up @@ -153,7 +156,7 @@ class MatcherExecutorSpec extends Specification {
@SuppressWarnings('LineLength')
def 'timestamp matcher'() {
expect:
MatcherExecutorKt.domatch(matcher, path, expected, actual, mismatchFactory).empty == mustBeEmpty
MatcherExecutorKt.domatch(matcher, path, expected, actual, mismatchFactory, false).empty == mustBeEmpty

where:
expected | actual | pattern || mustBeEmpty
Expand Down Expand Up @@ -181,7 +184,7 @@ class MatcherExecutorSpec extends Specification {
@Unroll
def 'time matcher'() {
expect:
MatcherExecutorKt.domatch(matcher, path, expected, actual, mismatchFactory).empty == mustBeEmpty
MatcherExecutorKt.domatch(matcher, path, expected, actual, mismatchFactory, false).empty == mustBeEmpty

where:
expected | actual | pattern || mustBeEmpty
Expand All @@ -196,7 +199,7 @@ class MatcherExecutorSpec extends Specification {
@Unroll
def 'date matcher'() {
expect:
MatcherExecutorKt.domatch(matcher, path, expected, actual, mismatchFactory).empty == mustBeEmpty
MatcherExecutorKt.domatch(matcher, path, expected, actual, mismatchFactory, false).empty == mustBeEmpty

where:
expected | actual | pattern || mustBeEmpty
Expand All @@ -211,7 +214,7 @@ class MatcherExecutorSpec extends Specification {
@Unroll
def 'include matcher matches if the expected is included in the actual'() {
expect:
MatcherExecutorKt.domatch(matcher, path, expected, actual, mismatchFactory).empty == mustBeEmpty
MatcherExecutorKt.domatch(matcher, path, expected, actual, mismatchFactory, false).empty == mustBeEmpty

where:
expected | actual || mustBeEmpty
Expand Down Expand Up @@ -241,19 +244,22 @@ class MatcherExecutorSpec extends Specification {
@Unroll
def 'list type matcher matches on array sizes - #matcher'() {
expect:
MatcherExecutorKt.domatch(matcher, path, expected, actual, mismatchFactory).empty == mustBeEmpty
MatcherExecutorKt.domatch(matcher, path, expected, actual, mismatchFactory, cascaded).empty == mustBeEmpty

where:
matcher | expected | actual || mustBeEmpty
TypeMatcher.INSTANCE | [0] | [1] || true
new MinTypeMatcher(1) | [0] | [1] || true
new MinTypeMatcher(2) | [0, 1] | [1] || false
new MaxTypeMatcher(2) | [0] | [1] || true
new MaxTypeMatcher(1) | [0] | [1, 1] || false
new MinMaxTypeMatcher(1, 2) | [0] | [1] || true
new MinMaxTypeMatcher(2, 3) | [0, 1] | [1] || false
new MinMaxTypeMatcher(1, 2) | [0, 1] | [1, 1] || true
new MinMaxTypeMatcher(1, 2) | [0] | [1, 1, 2] || false
matcher | expected | actual | cascaded || mustBeEmpty
TypeMatcher.INSTANCE | [0] | [1] | false || true
new MinTypeMatcher(1) | [0] | [1] | false || true
new MinTypeMatcher(2) | [0, 1] | [1] | false || false
new MinTypeMatcher(2) | [0, 1] | [1] | true || true
new MaxTypeMatcher(2) | [0] | [1] | false || true
new MaxTypeMatcher(1) | [0] | [1, 1] | false || false
new MaxTypeMatcher(1) | [0] | [1, 1] | true || true
new MinMaxTypeMatcher(1, 2) | [0] | [1] | false || true
new MinMaxTypeMatcher(2, 3) | [0, 1] | [1] | false || false
new MinMaxTypeMatcher(1, 2) | [0, 1] | [1, 1] | false || true
new MinMaxTypeMatcher(1, 2) | [0] | [1, 1, 2] | false || false
new MinMaxTypeMatcher(1, 2) | [0] | [1, 1, 2] | true || true
}

@Unroll
Expand Down Expand Up @@ -314,5 +320,4 @@ class MatcherExecutorSpec extends Specification {
xml('<foo xmlns="a"/>') || '<{a}foo>'
xml('<a>text</a>').firstChild || "'text'"
}

}
14 changes: 11 additions & 3 deletions core/matchers/src/test/groovy/au/com/dius/pact/core/matchers/MaximumMatcherSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class MaximumMatcherSpec extends Specification {
@Unroll
def 'with an array match if the array #condition'() {
expect:
MatcherExecutorKt.domatch(new MaxTypeMatcher(2), path, expected, actual, mismatchFactory).empty
MatcherExecutorKt.domatch(new MaxTypeMatcher(2), path, expected, actual, mismatchFactory, false).empty

where:
condition | expected | actual
Expand All @@ -29,7 +29,7 @@ class MaximumMatcherSpec extends Specification {
@Unroll
def 'with an array not match if the array #condition'() {
expect:
!MatcherExecutorKt.domatch(new MaxTypeMatcher(2), path, expected, actual, mismatchFactory).empty
!MatcherExecutorKt.domatch(new MaxTypeMatcher(2), path, expected, actual, mismatchFactory, false).empty

where:
condition | expected | actual
Expand All @@ -39,12 +39,20 @@ class MaximumMatcherSpec extends Specification {
@Unroll
def 'with a non array default to a type matcher'() {
expect:
MatcherExecutorKt.domatch(new MaxTypeMatcher(2), path, expected, actual, mismatchFactory).empty == beEmpty
MatcherExecutorKt.domatch(new MaxTypeMatcher(2), path, expected, actual, mismatchFactory, false).empty == beEmpty

where:
expected | actual || beEmpty
'Fred' | 'George' || true
'Fred' | 100 || false
}

def 'when cascaded, do not apply the limits'() {
given:
def expected = [1, 2]
def actual = [1, 2, 3]

expect:
MatcherExecutorKt.domatch(new MaxTypeMatcher(2), path, expected, actual, mismatchFactory, true).empty
}
}
Loading

0 comments on commit 2a12a32

Please sign in to comment.