Skip to content
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

Introduce assert() - an angry check() #3406

Open
netikras opened this issue Oct 19, 2023 · 7 comments
Open

Introduce assert() - an angry check() #3406

netikras opened this issue Oct 19, 2023 · 7 comments
Labels

Comments

@netikras
Copy link

netikras commented Oct 19, 2023

Feature Description

If a failed transaction's result is required for subsequent transactions in the iteration (e.g. if login() failed), there's no point in executing the rest of the txs. So we need a simple failure mechanism.

check() does not fail the iteration, it only records failures (if any).

I propose introducing a separate validation function, that does what check() does, except it fail()s instead of returning false.

Suggested Solution (optional)

  1. move the current check( val, sets, [tags] ) to checkOrFail( val, **fail: boolean**, sets, [tags] )
  2. create check( val, sets, [tags] ) that only does checkOrFail( val, false, sets, [tags] )
  3. create assert( val, sets, [tags] ) that only does checkOrFail( val, true, sets, [tags] )

In case an error is thrown, it should contain a list of failures, i.e.:

AssertionError: {
  failures: ['status is 200', 'contains JWT token']
}

I'm not that fluent in GO to implement it myself...yet. But I see where it should be changed:

https://github.com/grafana/k6/blob/master/js/modules/k6/k6.go

Already existing or connected issues / PRs (optional)

No response

@olegbespalov
Copy link
Contributor

Hi @netikras!

The assert function could be easily implemented in pure JavaScript and re-used in a project.

See an example below:

import http from 'k6/http';
import { check, fail } from 'k6';

const assert = (val, set, tags) => {
   if (!check(val, set, tags)) {
      fail('assertion failed');
   }
}

export default function () {
  const res = http.get('https://httpbin.test.k6.io');
  assert(
    res,
    {
      'response code was 200': (res) => res.status == 200,
      'body size was 1234 bytes': (res) => res.body.length == 1234,
    },
    { myTag: "I'm a tag" }
  );
}

Does that solve your need?

@netikras
Copy link
Author

@olegbespalov of course it could! I bet fail() also can be implemented in pure js, just as most of the other k6's features!

But since you created a fail("reason") shorthand for throw new Error("reason") claiming it's supposed to improve test code quality (make it cleaner, neater), it only feels right to implement similar shorthands/utilities for other often-used parts of the test, using the same reasons as justification: to improve code readability and maintainability. After all, it's supposed to look like a clean, simple test description and not an application code with all the if-else-throw blocks riddled all over the code.

Also, I think assert() is more beneficial than check() in perf testing, as it controls the flow's execution and makes test results less distorted by failing-fast an iteration if it's not supposed to continue.

And I see that assert() is a feature requested in k6's issues since long ago.

And your proposed solution doesn't exactly solve the problem, as (I think) it doesn't say which checks failed (body size? status code?)

I can continue my list of explanations on why I think assert() is a long-missing utility.. If you disagree, I'd gladly hear out your reasons against this utility.

@netikras
Copy link
Author

While we're at it, I think implementing assertThat() and checkThat() also makes sense - it would satisfy this request: #1066 and keep the backwards compatibility.

And IMO it sounds logical.

assert(req, assumptions) - reads "assert object with assumptions"
assertThat(assumption/s) - reads "assert that assumptions are true"

Same with check() and checkThat().

@olegbespalov
Copy link
Contributor

I can continue my list of explanations on why I think assert() is a long-missing utility.. If you disagree, I'd gladly hear out your reasons against this utility.

First of all, I'm trying to understand the need and the reason why you think that it should be implemented directly in k6 (golang). For example, we have a k6chaijs library that implements a BDD assertions approach using pure JavaScript https://k6.io/docs/javascript-api/jslib/k6chaijs/.

So my suggestion was just a quick example, but by using JavaScript you can try to create the method that serves your needs.

@netikras
Copy link
Author

@olegbespalov I'd say convenience and removing the necessity to always import a 3rd-party library in (almost) every script to write a simple e2e test flow with basic flow control, w/o reducing readability.
And in perf scripts, I consider iteration/flow control an essential feature of the tool.

Again, I'm referring to the fail() function

fail() is a simple convenience wrapper on top of JavaScript's throw(), because the latter cannot be used as [expr] || throw, which is a convenient way to write k6 test code.

https://k6.io/docs/javascript-api/k6/fail/

I understand that you are trying to avoid bloating the k6 code by excessive features.
However, I as a performance engineer, would very much like to have the basic perf script/test functions to come OOTB with the tool itself, w/o

  • searching for workarounds
  • reducing readability and maintainability of my test scripts
  • having to depend on 3rd party libraries in every script to do the same thing everywhere

HP LoadRunner has it built-in
JMeter has it built-in
BlazeMeter has it built-in
Gatling has it built-in

locust and k6 don't have it, unfortunately

@olegbespalov
Copy link
Contributor

Consider the diff

-import { assert } from 'k6';
+import { assert } from 'https://jslib.k6.io/k6-utils/2.3.0/index.js'; // this not yet exists

Or using a local module that implements an assert method.

So neither readability nor maintainability is actually dramatically decreasing. In the end, it is just about the difference in import.

Again, I'm referring to the fail() function

There is no need to focus on the fail() method if you want to abort the test fail's documentation suggests using the test.abort() https://k6.io/docs/javascript-api/k6-execution/#test

I understand that you are trying to avoid bloating the k6 code by excessive features.

Yes, you're right here. But also, there should be a significant reason (like performance implications) to have this implemented in the core/golang.

However, I, as a performance engineer, would very much like to have the basic perf script/test functions to come OOTB with the tool itself, w/o

I hear you and accept your needs, but I'm trying to point out that k6 can solve your needs already by using JavaScript, moreover, it could be tailored specially for you and your special case. And it could be implemented and used independently of the k6 release.

On another note, I fully agree that looking for workarounds could be frustrating. So at least, we could maybe try to provide a minimal example of the assert method that we could put to the check or fail methods.

@grafana/k6-core if you have something to add or have another opinion please chime in.

@olegbespalov olegbespalov removed their assignment Jan 9, 2024
@oleiade
Copy link
Member

oleiade commented Nov 26, 2024

ref #4067

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants