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
Issue #145, Minor issues fixed #146
Conversation
"SOURCE_CHANGE", | ||
"TIMER", | ||
"OTHER" | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
@@ -60,12 +60,12 @@ __Description:__ A list of metrics collected during the test case execution. Not | |||
|
|||
##### data.outcome.metrics.name | |||
__Type:__ String | |||
__Required:__ No | |||
__Required:__ Yes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the problem is we are now changing the definition of the event, so we need to step the version. This is because the .md file is the canonical source of truth: either we think 1.0.0 is correct and the schema is wrong, or it needs to be changed. Clearly, your change is the right one (data.outcome.metrics is optional, but if used its properties shouldn't be).
In other words, what is needed is stepping the version, e.g.:
| 1.0.1 | Current version | data.outcome.metrics.value and data.outcome.metrics.name made mandatory |
Then a 1.0.1 version of the schema needs to be added, and the current 1.0.0 version "fixed" (i.e. the properties made optional).
"SOURCE_CHANGE", | ||
"TIMER", | ||
"OTHER" | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
@@ -163,8 +163,7 @@ | |||
}, | |||
"required": [ | |||
"id", | |||
"testCase", | |||
"constraints" | |||
"testCase" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
@@ -24,7 +24,7 @@ How the test selection service generates the recipe collection is, from the poin | |||
|
|||
The __data__ object consists of two main parts. __data.selectionStrategy__ identifies the strategy used to select the test cases and generate the recipe collection, while __data.batches__ or __data.batchesUri__ contain or reference, respectively, the recipes. The recipes are grouped in batches, which are used to control the order of execution of test cases. Every batch has a priority to let the test executor order them in sequence, but within each batch no assumptions are made as to the execution order the test cases. This way the recipe collection can either allow the executor a high degree of freedom in scheduling the test executions, and/or prescribe the exact sequential order in which they must be executed. Each event SHALL include one and only one of __data.batches__ and __data.batchesUri__. | |||
|
|||
Finally, each recipe (__data.batches.recipes__) consists of two parts: the test case to execute, and the constraints of that execution. The EiffelTestExecutionRecipeCollectionCreatedEvent does control the syntax of these constraints, as the nature of such constraints are highly dependent on technology domain and test execution framework. That being said, there are three questions that typically need to be answered: what is the item under test, in what kind of environment is it to be tested, and what are the test parameters? Note the distinction between test case and test execution: it is perfectly legal for a single test case to appear multiple times within the same EiffelTestExecutionRecipeCollectionCreatedEvent, but (presumably) with different constraints. | |||
Finally, each recipe (__data.batches.recipes__) consists of two parts: the test case to execute, and the constraints of that execution. The EiffelTestExecutionRecipeCollectionCreatedEvent does not control the syntax of these constraints, as the nature of such constraints are highly dependent on technology domain and test execution framework. That being said, there are three questions that typically need to be answered: what is the item under test, in what kind of environment is it to be tested, and what are the test parameters? Note the distinction between test case and test execution: it is perfectly legal for a single test case to appear multiple times within the same EiffelTestExecutionRecipeCollectionCreatedEvent, but (presumably) with different constraints. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Thanks for making the pull request! Excellent stuff. The only problem as mentioned above is that we need to step the version of the TCF event. Would you be willing to do that? Let me know if you have questions. |
I will step the version of the TCF event and update the pull request |
@@ -153,6 +160,7 @@ | |||
"parameters": { | |||
"type": "array", | |||
"items": { | |||
"type": "object", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
@@ -14,7 +14,7 @@ | |||
}, | |||
"version": { | |||
"type": "string", | |||
"enum": [ "1.0.0" ], | |||
"enum": [ "1.0.0", "1.0.1" ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@d-stahl-ericsson Could you please check this changes also
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't want to rename 1.0.0.json to 1.0.1.json. Instead, you want to copy it. 1.0.0.json should only check 1.0.0 of the event (i.e. "enum": [ "1.0.0" ]
) and 1.0.1.json should only check 1.0.1 of the event (i.e. "enum": [ "1.0.1" ]
). The former will not require data.outcome.metrics properties, but the latter will.
@@ -86,7 +86,7 @@ __Description:__ The URI at which the log can be retrieved. | |||
## Version History | |||
| Version | Introduced in | Changes | | |||
| --------- | ------------------------------------------------------ | --------------------------------------- | | |||
| 1.0.0 | [edition-bordeaux](../../../tree/edition-bordeaux) | Initial version. | | |||
| 1.0.1 | [edition-bordeaux](../../../tree/edition-bordeaux) | Current version. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I didn't spot this one before. This should be a history of all versions, not just the latest. In other words,
| 1.0.1 | Current version | data.outcome.metrics.value and data.outcome.metrics.name made mandatory. |
| 1.0.0 | [edition-bordeaux](../../../tree/edition-bordeaux) | Initial version. |
Looks good to me. Can we get one more pair of eyes on this, please? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, Looks good to me.
No description provided.