[DRIVERS-1652] Clarify events.json and result.json produced by workload executor Created: 07/Apr/21 Updated: 01/Sep/21 |
|
| Status: | Implementing |
| Project: | Drivers |
| Component/s: | Atlas Testing |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Jeremy Mikola | Assignee: | Jeremy Mikola |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Driver Changes: | Needed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Driver Compliance: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
This issue has been repurposed to clarify expected output for events.json and results.json in all cases:
These changes will be incorporated into the workload executor spec document and the validation tests will be revised accordingly. For additional context, this issue's original summary and description follows: Incorrect assertions for result.json in ValidateWorkloadExecutorBehavioral Description: #4 states:
If the loop operation does not store errors or failures in an entity, those exceptions are expected to interrupt the loop and propagate to the test runner. My understanding of the text above is that the workload executor is expected to capture and report those errors in the same manner – and thus product results.json. I'm confused as to why the validator-numFailures-not-captured.yml test expects -1 to be reported for numFailures in that case (and likewise for validator-numErrors-not-captured.yml). If anything, ValidateWorkloadExecutor's.test_num_failures_not_captured should expect 1 for numFailures (and possibly -1 for numErrors) since an uncaught failure will abort the loop on its first iteration and the workload executor should report that single failure on its own. Behavior Description: #8 states:
This does not agree with the expectations for ValidateWorkloadExecutor's "simple test", which does not specify storeErrorsAsEntity or storeFailuresAsEntity but later asserts that results.json does not have any -1 values. Per the quoted text, a workload executor should use -1 for the error/failure counts. This issue extends to other ValidateWorkloadExecutor tests, which specify either storeErrorsAsEntity or storeFailuresAsEntity (but not both). For example, validator-numErrors.yml and validator-numFailures-as-errors.yml only use storeErrorsAsEntity. I would expect results.json to produce an actual count for numErrors (based on the size of the errors array in events.json) and leave numFailures unset (i.e. -1); however, this conflicts with the assertions in ValidateWorkloadExecutor.run_test:
I'm not sure what needs to change here, nor do I understand how existing implementations are currently passing the ValidateWorkloadExecutor tests. Does the documented behavior for the workload executor need to be changed, or are the assertions in ValidateWorkloadExecutor incorrect? |
| Comments |
| Comment by Githook User [ 21/Apr/21 ] |
|
Author: {'name': 'Jeremy Mikola', 'email': 'jmikola@gmail.com', 'username': 'jmikola'}Message: DRIVERS-1652: Clarify events.json and result.json produced by workload executor (#120)
This rewrites portions of the workload executor spec to clearly define how events.json and results.json are produced. This also improves instructions for when the test runner propagates and error/failures, which must then be reported by the workload executor in the same format.
Removes the placeholder stats if results.json cannot be read. Per the spec, the workload executor should always produce results.json (and events.json). Always assert presence and types of required fields in results.json and events.json. Improve events.json assertions for errors and failures. Add numErrors-as-failures test.
Co-authored-by: Andreas Braun <alcaeus@users.noreply.github.com> |
| Comment by Jeremy Mikola [ 14/Apr/21 ] |
|
https://github.com/mongodb-labs/drivers-atlas-testing/pull/120 |