Add config option `throwAssertionError`. Default to false

ppcano commented 2 years ago

The expect assertion API is more flexible and feature-rich than the k6 Check API. It is a great addition to k6, but k6chaijs does not follow the behavior of Check assertions.

Ideally, expect should be a valid BDD alternative to Check. But it is not possible now.

Today, expect and describe are coupled. The reason is that when an expect assertion fails, it exits the execution by triggering an AssertionError exception. And, describe is responsible for handling the AssertionErrors. Thus, expect assertions do not behave like Checks:

throw an exception on failures
and therefore, should be called within describe functions

If expect does not throw the AssertionError on failures. expect will become a Check alternative. Users could replace their checks with expects without further changes to the test script logic.

I believe we'll increase the adoption of this library by positioning it as a BDD alternative to Check.

use expect or check
not need to use anything else or change the way you script your k6 test

And then, not as the "initial" primary case, we'll introduce other use cases for this library.

This PR adds the throwAssertionError option to toggle this behavior. By default, it is False to behave like Check.

Below are some examples to visualize how the new option works:

Default

import http from "k6/http";
import chai, { expect } from "../k6-jslib-k6chaijs/build/k6chaijs.min.js";

export default function () {
    expect({ a: 1 }).to.not.have.property("a");
    expect(2).to.equal(2);
}

  scenarios: (100.00%) 1 scenario, 1 max VUs, 10m30s max duration (incl. graceful stop):
           * default: 1 iterations for each of 1 VUs (maxDuration: 10m0s, gracefulStop: 30s)

running (00m00.0s), 0/1 VUs, 1 complete and 0 interrupted iterations
default ✓ [======================================] 1 VUs  00m00.0s/10m0s  1/1 iters, 1 per VU

     ✗ expected ${this} to not have property 'a'
      ↳  0% — ✓ 0 / ✗ 1
     ✓ expected ${this} to equal 2

Previous behaviour - enabling throwAssertionError

import http from "k6/http";
import chai, { expect } from "../k6-jslib-k6chaijs/build/k6chaijs.min.js";

chai.config.throwAssertionError = true;

export default function () {
    expect({ a: 1 }).to.not.have.property("a");
    expect(2).to.equal(2);
}

  scenarios: (100.00%) 1 scenario, 1 max VUs, 10m30s max duration (incl. graceful stop):
           * default: 1 iterations for each of 1 VUs (maxDuration: 10m0s, gracefulStop: 30s)

ERRO[0000] AssertionError: expected { a: 1 } to not have property 'a'
    at file:///Users/ppcano/dev/li/k6-tests/k6-jslib-k6chaijs/build/k6chaijs.min.js:4:54237(162)
    at apply (native)
    at r (file:///Users/ppcano/dev/li/k6-tests/k6-jslib-k6chaijs/build/k6chaijs.min.js:4:10289(36))
    at proxy (native)
    at de (file:///Users/ppcano/dev/li/k6-tests/k6-jslib-k6chaijs/build/k6chaijs.min.js:4:26989(228))
    at apply (native)
    at e (file:///Users/ppcano/dev/li/k6-tests/k6-jslib-k6chaijs/build/k6chaijs.min.js:4:9422(19))
    at proxy (native)
    at file:///Users/ppcano/dev/li/k6-tests/chaijs/expect-test.js:10:28(12)
    at native  executor=per-vu-iterations scenario=default source=stacktrace

running (00m00.0s), 0/1 VUs, 1 complete and 0 interrupted iterations
default ✓ [======================================] 1 VUs  00m00.0s/10m0s  1/1 iters, 1 per VU

     ✗ expected ${this} to not have property 'a'
      ↳  0% — ✓ 0 / ✗ 1

2Steaks commented 2 years ago

It seems we all have quite different ideas about how this library should work 😄

I don't think I have much to add to this discussion, other than I believe adoption would be greater from the Javascript community if we followed the standard describe, test/it, expect pattern. Frameworks using this pattern do not work without using all 3, which is expected.

However, that would change how it's currently implemented:

group -> describe (rename only)
check -> test/it (this is where we catch the error)
expect just throws

describe('just a group', () => {

    // this is the text that would display in the output for passes/failures NOT the `expect` function
    test('should fetch a list of public crocodiles', () => {

         const response = http.get('https://test-api.k6.io/fail/');
         expect(response.status, 'response status').to.equal(200); // throws only
         expect(response.json().length, 'number of crocs').to.be.above(4);
    })
})

If we want developers to shift left, there is barely anything for them to learn with this pattern.

ppcano commented 2 years ago

I am ok with both arguments and the three-level pattern.

I suggest starting to provide a compatible assertion API because 99% of k6 examples use the Check API, and we'll continue showing examples with k6 Core APIs.

The adoption will be more difficult if we force you to change the way you implement your scripts. I suggested starting facilitating that adoption and highlighting the other case as an additional case. We can learn from these users who intentionally selected this approach.

Anyway, I think the improvements of this library are not only related to particular testing types: API testing or functional testing.

If we find it valuable to throw assertion errors + error-safe "group/tests" - these improvements should be on the k6 core APIs. Perhaps, better to spend time changing it to k6 API than with projects that might have little visibility.

grafana / k6-jslib-k6chaijs

Add config option `throwAssertionError`. Default to false #8