feat: script validation

[Llandy3d]

Logic for Validating a k6 script.

Group info

To retrieve group information there is a snippet of javascript that will be added to the script after the http client is imported. This snippet replaces the client with a wrapper that will add the X-k6-Group header containing group information.
This approach is similar to what the tracing instrumentation module does.

This group information is then extracted by the proxy before making the request and stored in the comment field of the ProxyData datastructure.

Events

There are three events added:

• script:select - opens a dialog to select a script locally and returns the path to it
• script:run
• script:stop

The name of these could be improved.

Requests

For requests the approach is the same as with the Recorder, the proxy captures and send the events about requests/responses in the same way, with the added group info in the comment field.

[Llandy3d]

as the script is a snippet to be added to a k6 script I'm ignoring normal validation for it

[Llandy3d]

this is a first draft of the approach, this gets inserted in the user script by us.
Right now it's dumb and it's just looking for a line k6/http and inserts this code after it.

[Llandy3d]

using comment, if we need to pass more information this could be a json string, but I'm keeping it simple for now

[Llandy3d]

we don't actually get debug logs, those are only emitted when k6 is run with the -v flag

[Llandy3d]

we are running the locally installed k6 in this pr

[Llandy3d]

the modified script, enhanced with our snippet is stored locally on a temp file, a cons of this approach is that when an errors happens it will showcase that path instead of the original script path 🤔

[Llandy3d]

by default k6 logs are printed in stderr 🤷

[going-confetti]

I haven't tested it just yet, but added a few comments/suggestions around mutation of function arguments, which IMO we should avoid.

[going-confetti]

I would prefer not mutating the args.

Suggested change

	request(method, url, ...args) {
	const group = { 'X-k6-group': trimAndRemovePrefix(execution.vu.tags.group) }
	args = instrumentArguments(group, ...args)
	return request(method, url, ...args)
	}
	request(method, url, ...args) {
	const group = { 'X-k6-group': trimAndRemovePrefix(execution.vu.tags.group) }
	return request(method, url, instrumentArguments(group, args)
	)
	}

This suggestion would require instrumentArguments to accept args as a single parameter rather than a spread of potentially multiple params. I will provide more detail in a separate comment.

[going-confetti]

This can as well be replaced with

function instrumentArguments(groupName, args) {
  // ...
}

As long as the function calls are updated accordingly

[going-confetti]

Let's avoid mutating function arguments here as well. I'll post a suggested change in a bit (initially misinterpreted the code and wrote a snippet that wouldn't work)

[going-confetti]

This is my suggestion (it needs to be tested though):

function instrumentArguments(groupName, args) {
  switch (args.length) {
    case 0:
      return [null, { headers: groupName }];
    case 1:
      return [args[0], { headers: groupName }];
    default:
      return args.map((arg, i) => {
        if (i !== 1) return arg
        
        return !arg ? { headers: groupName } : { headers: {...arg.headers, groupName }}
      })
  }
}

Could you also make a comment explaining what this function does?

[Llandy3d]

note: I will be extracting this as a separate ticket to fully cleanup the draft of the group snippet 🙌

[e-fisher]

Agree with Uladzimir's comments, also added a comment about extracting resource reader function

[e-fisher]

This logic to read resources is repeated in a few places, could be extracted into helper function:

function getResource(resource: string) { 
  if (MAIN_WINDOW_VITE_DEV_SERVER_URL) {
    return path.join(app.getAppPath(), 'resources', resource)
  } else {
    return path.join(process.resourcesPath, resource)
  }
}

[Llandy3d]

my rule of thumbs is 3 occurrences for extracting common behaviour, here we should also handle the case where the path is built also depending on os/architecture 🤔

[Llandy3d]

It's also not reusable in every place because when there are multiple paths, it behaves differently between the local resource and the packaged app, there are some caveats.
Because of that I have a preference to have them explicit for now and consolidate when we know all the resources we need and refactor that

[e-fisher]

LGTM!