refactor: correlation rule #85
Conversation
| correlationExtractionSnippet: correlationExtractionSnippet, | ||
| generatedUniqueId: uniqueId, | ||
| } | ||
| } | ||
|
|
||
| // @ts-expect-error we have commonjs set as module option | ||
| if (import.meta.vitest) { |
There was a problem hiding this comment.
I've used in-source tests as they were useful to test things without exporting while refactoring, we might not want to use them since it's not standard but let's discuss it!
This reverts commit 44628e9affdc90d55a113c629e528ac1df7c5530.
Llandy3d
force-pushed
the
correlation_refactor
branch
from
c433183 to
a945e60
Compare
src/rules/correlation.ts
Outdated
| ) => { | ||
| // TODO: replace regex with findBetween from k6-utils once we have imports | ||
| return ` | ||
| regex = new RegExp('${rule.extractor.selector.begin}(.*?)${rule.extractor.selector.end}') |
There was a problem hiding this comment.
What if we hard-code the import and then add a prettier plugin that removes unused imports for cases when it's not used?
There was a problem hiding this comment.
I still think it might be better to figure out imports first because we could also have situations where the import might not be reachable via network. (closed down environments)
Also most likely we are going to need to implement our own version of findBetween that supports not specifying an end. 🤔
There was a problem hiding this comment.
I still think it might be better to figure out imports first because we could also have situations where the import might not be reachable via network. (closed down environments)
We won't be able to use jslib in such cases unless we ship local copies of those libraries along with k6 studio - is this something we want to do?
There was a problem hiding this comment.
not yet decided but is something that we will need to consider, talking with Tom in closed down environments making a local copy of the libraries is the only option (and what was done so far)
There was a problem hiding this comment.
That task is getting more and more involved with each passing day it seems :D
There was a problem hiding this comment.
indeed, we should probably try to split it into multiple parts 🤔
There was a problem hiding this comment.
I've created a PR to remove type casts: #88
The reason you needed type casts was switching on nested property but passing the whole rule object, while only selector was used
Also, would you mind moving extraction functions into separate files? I think it would be so much easier to review and maintain if one file consisted of only the function tested and tests that belong to it.
I actually used that approach for For removing extraction functions into separate files that's a yes but not in this PR since there is another one depending on it and I was already thinking of doing that while implementing |
Closes https://github.com/grafana/k6-cloud/issues/2550
Adds new specific types removing a lot of unneeded checks and add tests.