feat: Insert test variables and sleeps in script #56
Conversation
src/constants/generator.ts
Outdated
There was a problem hiding this comment.
Removed for consistency: we don't use enums in other places (for example, for rule types, sleep types, etc.)
| case 'fixed': | ||
| return `sleep(${timing.value})` | ||
| case 'range': | ||
| return `sleep(Math.random() * (${timing.value.max} - ${timing.value.min}) + ${timing.value.min})` |
There was a problem hiding this comment.
We should switch to randomIntBetween once we've figured out imports
| setPreview(script) | ||
| } | ||
|
|
||
| updatePreview() | ||
| // TODO: implement a more reactive way to update the preview |
There was a problem hiding this comment.
Maybe we should use this middleware to subscribe to store changes. I'll think about this in my future PRs
| export function generateVariableDeclarations(variables: Variable[]): string { | ||
| return variables | ||
| .filter(({ name }) => name) | ||
| .map(({ name, value }) => `const ${name} = "${value}"`) |
There was a problem hiding this comment.
I think we might want to namespace variables ?
possibly a global VARS object or similar to have them bounded, what do you think ?
There was a problem hiding this comment.
I remember we discussed this, but noticed that we didn't follow this approach for correlated values - do you remember why?
In this case, one potential downside of this would be explaining to the user that variables need to be accessed not just by their names, but also by the name of the test variables object. It may still lead to a better DX in Monaco
There was a problem hiding this comment.
more than a downside it might be a perk, that variables defined will be always autocompleted from VARS or similar 🤔
but noticed that we didn't follow this approach for correlated values - do you remember why?
yap, correlation runs at runtime so we define the variable before extracting it and use it with the sequentially incremented idcorrelation_1
For variables, we know about them when generating the script and there are advantages in grouping them under a common name.
In theory you could have an object also for correlation variables but we don't have problems with them at script level because of the autoincrementing id that we do not make use of in vars
| rules: generatorState.rules, | ||
| allowlist: generatorState.allowList, | ||
| } | ||
| const generatorFile = storeToGeneratorFileData(useGeneratorStore.getState()) |
I agree. Mayne we can change the interaction in the following way:
Although I admit it may be confusing. Still, putting requests into a single group doesn't feel right somehow :\ |
yeah I completely agree that it does not feel right at all. I think your proposal makes sense, specially if we consider improvements in the future where you can pick which groups to keep in the recording before saving the HAR if it makes sense at all 🤔 |
No description provided.