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.

Sign up for GitHub

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

Jump to bottom

Add indexes to test names so that Specmatic always generates tests with unique names. #1082

Closed
wants to merge 9 commits into from
Closed

Add indexes to test names so that Specmatic always generates tests with unique names. #1082

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
afd8063
If multiple tests with same name are generated, they will get suffixe…
jaydeepk Apr 25, 2024
5239dd7
Added tests to multiple and singular test names.
jaydeepk Apr 29, 2024
4cb8d40
Fixed issue with incorrect response codes in test names for negative …
burdeamit Apr 29, 2024
3f276bd
Fixed the issue that will arise when using --filter-name argument to …
burdeamit Apr 30, 2024
8f13d7c
Removed unused imports
burdeamit Apr 30, 2024
b5db049
Restored the test: positive examples of 4xx should be able to have no…
burdeamit May 1, 2024
93d1ade
Renamed the variable in negativeTestScenarios fun to more descriptive…
burdeamit May 1, 2024
c079382
Renamed variable 'negativeTestScenarioR' to 'currentNegativeTestScena…
burdeamit May 1, 2024
3aa0303
Renamed addIndex method for more clarity
burdeamit May 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions core/src/main/kotlin/in/specmatic/core/Feature.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,8 @@ data class Feature(
flagsBased
else
flagsBased.withoutGenerativeTests()

originalScenario.generateTestScenarios(resolverStrategies, testVariables, testBaseURLs).map { Pair(originalScenario.copy(generativePrefix = flagsBased.positivePrefix), it) }
val testScenarios = originalScenario.generateTestScenarios(resolverStrategies, testVariables, testBaseURLs)
addIndexes(originalScenario, testScenarios, flagsBased.positivePrefix)
}

fun negativeTestScenarios(): Sequence<Pair<Scenario, ReturnValue<Scenario>>> {
Expand All @@ -365,19 +365,13 @@ data class Feature(
val negativeTestScenarios =
negativeScenario.generateTestScenarios(flagsBased, testVariables, testBaseURLs)

negativeTestScenarios.filterNot { negativeTestScenarioR ->
val filtered = negativeTestScenarios.filterNot { negativeTestScenarioR ->
Copy link
Member

Choose a reason for hiding this comment

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

Can we please rename filtered to what it actually contains? This will help readability of line where filtered is being used?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in: Renamed the variable in negativeTestScenarios fun to more descriptive…

Copy link
Member

Choose a reason for hiding this comment

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

not sure I understand why we are naming this negativeTestScenarioR? what is the R? Can you please check?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • negativeTestScenarioR is essentially a temporary variable used to hold each individual negative test scenario as it's being evaluated by the filterNot function.
  • This will be renamed to currentNegativeTestScenario. So the name indicates that this is the current negative test scenario being evaluated in the filter operation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in : Renamed variable 'negativeTestScenarioR' to 'currentNegativeTestScena…

negativeTestScenarioR.withDefault(false) { negativeTestScenario ->
val sampleRequest = negativeTestScenario.httpRequestPattern.generate(negativeTestScenario.resolver)
originalScenario.httpRequestPattern.matches(sampleRequest, originalScenario.resolver).isSuccess()
}
}.mapIndexed { index, negativeTestScenarioR ->
Pair(negativeScenario, negativeTestScenarioR.ifValue { negativeTestScenario ->
negativeTestScenario.copy(
generativePrefix = flagsBased.negativePrefix,
disambiguate = { "[${(index + 1)}] " }
)
})
}
addIndexes(negativeScenario, filtered, flagsBased.negativePrefix)
}
}

Expand All @@ -400,6 +394,27 @@ data class Feature(
serverState = emptyMap()
}

private fun addIndexes(
Copy link
Member

Choose a reason for hiding this comment

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

Technically this method is adding both +v/-ve prefix as well as index, should it be named appropriately?

Copy link
Member

Choose a reason for hiding this comment

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

Will also be super helpful to unit test this method separately. While the feature level tests are very useful, this will allow is try all difficult combinations.

Copy link
Member

Choose a reason for hiding this comment

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

Is this method doing too much, should we separate out application of prefix and index as two separate collections.map operations?

Copy link
Member Author

Choose a reason for hiding this comment

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

ToDo's

  • Technically this method adds both the +v/-ve prefix and the index, should it be named appropriately?
  • It will also be super helpful to unit test this method separately. While the feature-level tests are very useful, this will allow us to try all difficult combinations.
  • Is this method doing too much, should we separate the application of prefix and index as two separate collections.map operations?

originalScenario: Scenario,
testScenarios: Sequence<ReturnValue<Scenario>>,
prefix: String
): Sequence<Pair<Scenario, ReturnValue<Scenario>>> {
val sequenceHasZeroOrOne = testScenarios.take(2).count() < 2

return testScenarios.mapIndexed { index, positiveTestScenarioR ->
val indexLabel: () -> String = if (sequenceHasZeroOrOne) {
{ "" }
} else {
{ "[${(index + 1)}] " }
}
Pair(
originalScenario.addIndexLabelAndPrefix(prefix, indexLabel),
positiveTestScenarioR.ifValue { scenario ->
scenario.addIndexLabelAndPrefix( prefix, indexLabel)
})
}
}

private fun combine(baseScenario: Scenario, newScenario: Scenario): Scenario {
return convergeURLMatcher(baseScenario, newScenario).let { convergedScenario ->
convergeHeaders(convergedScenario, newScenario)
Expand Down
12 changes: 10 additions & 2 deletions core/src/main/kotlin/in/specmatic/core/Scenario.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ data class Scenario(
val serviceType:String? = null,
val generativePrefix: String = "",
val statusInDescription: String = httpResponsePattern.status.toString(),
val disambiguate: () -> String = { "" }
val indexLabel: () -> String = { "" }
): ScenarioDetailsForResult {
constructor(scenarioInfo: ScenarioInfo) : this(
scenarioInfo.scenarioName,
Expand Down Expand Up @@ -430,7 +430,7 @@ data class Scenario(

val generativePrefix = this.generativePrefix

return "$generativePrefix Scenario: $method $path ${disambiguate()}-> $statusInDescription$exampleIdentifier"
return "$generativePrefix Scenario: $method $path ${indexLabel()}-> $statusInDescription$exampleIdentifier"
}

fun newBasedOn(scenario: Scenario): Scenario {
Expand Down Expand Up @@ -485,6 +485,14 @@ data class Scenario(
&& operationId.responseStatus == status
&& httpRequestPattern.matchesPath(operationId.requestPath, resolver).isSuccess()
}

fun addIndexLabelAndPrefix(
prefix: String,
indexLabel: () -> String
) = this.copy(
generativePrefix = prefix,
indexLabel = indexLabel
)
}

fun newExpectedServerStateBasedOn(
Expand Down
187 changes: 130 additions & 57 deletions core/src/test/kotlin/in/specmatic/core/FeatureTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1482,7 +1482,7 @@ paths:
}

@Test
fun `positive examples of 4xx should be able to have non-string non-spec-conformant examples`() {
harikrishnan83 marked this conversation as resolved.
Show resolved Hide resolved
fun `multiple positive tests with same names should have an index suffix`() {
val specification = OpenApiSpecification.fromYAML("""
openapi: 3.0.0
info:
Expand All @@ -1494,74 +1494,147 @@ servers:
description: Local
paths:
/products:
post:
summary: Add Product
description: Add Product
requestBody:
content:
application/json:
schema:
type: object
required:
- name
properties:
name:
type: string
examples:
SUCCESS:
value:
name: 'abc'
BAD_REQUEST_NUMBER:
value:
name: 10
BAD_REQUEST_NULL:
value:
name: null
get:
summary: Search for products
description: get-products
parameters:
- name: type
in: query
schema:
type: string
enum:
- gadget
- book
- food
- other

responses:
'200':
description: Returns Id
description: List of products in the response
content:
text/plain:
schema:
type: string
examples:
SUCCESS:
value: 10
'422':
description: Bad Request
type: string
""".trimIndent(), "").toFeature()

val contractTests = specification.generateContractTestScenarios(emptyList())
val testNames = contractTests.map { it.first.testDescription().trim() }.toList()
assertThat(testNames).isEqualTo(
listOf(
"Scenario: GET /products [1] -> 200",
"Scenario: GET /products [2] -> 200",
"Scenario: GET /products [3] -> 200",
"Scenario: GET /products [4] -> 200",
"Scenario: GET /products [5] -> 200",
)
)
}

@Test
fun `positive tests with distinct names should not have an index suffix`() {
val specification = OpenApiSpecification.fromYAML("""
openapi: 3.0.0
info:
title: Sample Product API
description: Optional multiline or single-line description in [CommonMark](http://commonmark.org/help/) or HTML.
version: 0.1.9
servers:
- url: http://localhost:8080
description: Local
paths:
/products:
get:
summary: Search for products
description: get-products
parameters:
- name: type
in: query
schema:
type: string
required: true
responses:
'200':
description: List of products in the response
content:
text/plain:
schema:
type: string
/orders:
get:
summary: Search for orders
description: get-orders
parameters:
- name: status
in: query
schema:
type: string
required: true

responses:
'200':
description: List of orders in the response
content:
text/plain:
schema:
type: string
""".trimIndent(), ""
).toFeature()

val contractTests = specification.generateContractTestScenarios(emptyList())
val testNames = contractTests.map { it.first.testDescription().trim() }.toList()
assertThat(testNames).isEqualTo(
listOf(
"Scenario: GET /products -> 200",
"Scenario: GET /orders -> 200"
)
)
}

@Test
fun `multiple negative tests with same names should have an index suffix`() {
val specification = OpenApiSpecification.fromYAML("""
openapi: 3.0.0
info:
title: Sample Product API
description: Optional multiline or single-line description in [CommonMark](http://commonmark.org/help/) or HTML.
version: 0.1.9
servers:
- url: http://localhost:8080
description: Local
paths:
/products:
get:
summary: Search for products
description: get-products
parameters:
- name: type
in: query
schema:
type: string
enum:
- gadget
- book
required: true
responses:
'200':
description: List of products in the response
content:
text/plain:
schema:
type: string
examples:
BAD_REQUEST_NUMBER:
value: "Bad request was received and could not be handled"
BAD_REQUEST_NULL:
value: "Bad request was received and could not be handled"
""".trimIndent(), "").toFeature()

val results = specification.executeTests(object : TestExecutor {
override fun execute(request: HttpRequest): HttpResponse {
print(request.toLogString())
return when(val body = request.body) {
is JSONObjectValue -> {
if(body.jsonObject["name"] is StringValue) {
HttpResponse.ok("10")
} else {
HttpResponse(422, "Bad request was received and could not be handled")
}
}

else -> HttpResponse(422, "Bad request was received and could not be handled")
}
}

override fun setServerState(serverState: Map<String, Value>) {
}
})
val contractTests = specification.enableGenerativeTesting().generateContractTestScenarios(emptyList())
val testNames = contractTests.map { it.first.testDescription().trim() }.toList()

assertThat(results.success()).isTrue()
assertThat(results.failureCount).isZero()
assertThat(testNames).isEqualTo(
listOf(
"+ve Scenario: GET /products [1] -> 200",
"+ve Scenario: GET /products [2] -> 200",
"-ve Scenario: GET /products [1] -> 4xx",
"-ve Scenario: GET /products [2] -> 4xx"
)
)
}

@Test
Expand Down