[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:
Duplicate
Issue split
split to PHPLIB-714 Clarify events.json and result.json p... Closed
split to RUBY-2587 Update Atlas workload executor for re... Closed
split to CSHARP-3826 Clarify events.json and result.json p... Backlog
split to CXX-2363 Clarify events.json and result.json p... Backlog
split to GODRIVER-2144 Clarify events.json and result.json p... Backlog
split to NODE-3584 Clarify events.json and result.json p... Backlog
split to JAVA-4114 Update Atlas workload executor for re... Backlog
split to CDRIVER-4144 Clarify events.json and result.json p... Closed
split to MOTOR-816 Clarify events.json and result.json p... Closed
split to PYTHON-2892 Clarify events.json and result.json p... Closed
split to RUST-1010 Clarify events.json and result.json p... Closed
Related
Driver Changes: Needed
Driver Compliance:
Key Status/Resolution FixVersion
JAVA-4114 Backlog
RUBY-2587 Done
CDRIVER-4144 Won't Do
CXX-2363 Backlog
CSHARP-3826 Backlog
GODRIVER-2144 Backlog
NODE-3584 Backlog
MOTOR-816 Duplicate
PYTHON-2892 Duplicate
PHPLIB-714 Duplicate
RUST-1010 Duplicate
SWIFT-1342 Won't Do

 Description   

This issue has been repurposed to clarify expected output for events.json and results.json in all cases:

  • results.json and events.json should always be created by the workload executor (even if the test runner propagates an error not caught in a loop)
  • events.json must always report arrays for events, errors, and failures. Those arrays may be empty. If any events, errors, or failures are obtained from the entity map, they will be appended to those arrays. If the unified test runner propagates an error (e.g. not caught by a loop), the workload executor is expected to report the error/failure and append it to an array accordingly.
  • numErrors and numFailures in results.json will always report the size of the respective array in events.json and never be unset (-1).
  • numSuccesses and numIterations in results.json may be unset (-1) if their respective values cannot be obtained from the entity map.

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 ValidateWorkloadExecutor

Behavioral Description: #4 states:

If the unified test runner raises an error while executing the workload, the error MUST be reported using the same format as errors handled by the unified test runner, as described in the unified test runner specification under the loop operation. Errors handled by the workload executor MUST be included in the calculated (and reported) error count.

If the unified test runner reports a failure while executing the workload, the failure MUST be reported using the same format as failures handled by the unified test runner, as described in the unified test runner specification under the loop operation. Failures handled by the workload executor MUST be included in the calculated (and reported) failure count. If the driver’s unified test runner is intended to handle all failures internally, failures that propagate out of the unified test runner MAY be treated as errors by the workload executor.

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:

MUST calculate the aggregate counts of errors (numErrors) and failures (numFailures) from the error and failure lists. If the errors or failures were not reported by the test runner, such as because the respective options were not specified in the test scenario, the workload executor MUST use -1 as the value for the respective counts.

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:

        if any(val < 0 for val in stats.values()):
            self.fail("The workload executor reported incorrect execution "
                      "statistics. Reported statistics MUST NOT be negative.")

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.

  • DRIVERS-1652: Make ValidateWorkloadExecutor consistent with spec

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.

  • Always remove events.json before spawning workload executor
  • Fix wrong comment wording

Co-authored-by: Andreas Braun <alcaeus@users.noreply.github.com>
Branch: master
https://github.com/mongodb-labs/drivers-atlas-testing/commit/481a6c0b3323280276d593cf5cf54e26958ec6f2

Comment by Jeremy Mikola [ 14/Apr/21 ]

https://github.com/mongodb-labs/drivers-atlas-testing/pull/120

Generated at Thu Feb 08 08:23:46 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.