Add "timer" option to JSON and JUnit formatters #1681
Conversation
features/editor_url.feature
Outdated
| """ | ||
|
|
||
| Scenario: Editor URL does not affect JSON formatter paths | ||
| Given I clear the default behat options |
There was a problem hiding this comment.
By default all our Behat tests are run with the "timer" option of the formatter set to false (so that timer output does not pollute what we are checking), and since this will now affect the output of the JSON and JUnit formatters, we need to disable this default option in several places
There was a problem hiding this comment.
This is fine as-is, but I would actually be tempted to go the other way, and remove all the time: -IGNORE-VALUE- / time="-IGNORE-VALUE-" from the JSON/JUnit examples in features where it's not relevant (e.g. just keeping it in the main json_format and junit_format feature files).
I think having this new option is an opportunity to make our feature files more robust / less confusing by reducing the number of places we need to implement magic to translate the specification to / from the actual output?
| "type": "object", | ||
| "additionalProperties": false, | ||
| "required": ["tests", "skipped", "failed", "pending", "undefined", "time", "suites"], | ||
| "required": ["tests", "skipped", "failed", "pending", "undefined", "suites"], |
There was a problem hiding this comment.
The "time" property is now optional
7fbefcb to
b6a7626
Compare
acoulton
left a comment
There was a problem hiding this comment.
This is great @carlos-granados, just a small suggestion on the feature files for the unrelated features.
features/editor_url.feature
Outdated
| """ | ||
|
|
||
| Scenario: Editor URL does not affect JSON formatter paths | ||
| Given I clear the default behat options |
There was a problem hiding this comment.
This is fine as-is, but I would actually be tempted to go the other way, and remove all the time: -IGNORE-VALUE- / time="-IGNORE-VALUE-" from the JSON/JUnit examples in features where it's not relevant (e.g. just keeping it in the main json_format and junit_format feature files).
I think having this new option is an opportunity to make our feature files more robust / less confusing by reducing the number of places we need to implement magic to translate the specification to / from the actual output?
The other usecase I guess (e.g. with the junit) is if you have tooling aggregating results across all builds - it can be useful to spot performance regressions / improvements over time. But I agree that having the option to disable it is very helpful. |
|
@acoulton good shout about removing the timer info from all tests where it is not relevant, I updated them |
acoulton
left a comment
There was a problem hiding this comment.
Great, thanks @carlos-granados
The information that shows the time spent to run each test in the JSON and JUnit formatters is interesting if you are trying to optimise the performance of your tests but in many other cases you may only be interested in whether the tests pass or not and this timer information will be of no interest to you. This PR adds a "timer" option to these formatters (similar to the one already available in the pretty and progress formatters) that can be used to disable the output of this data. It defaults to true to preserve existing functionality