-
Notifications
You must be signed in to change notification settings - Fork 33
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
Implement converter #2
Conversation
The architecture has the CLI tool in a different package.
No longer needed after factoring validation.
Didn't realize it would be used in the browser. Is that a requirement? Think I'd have to revisit the compatibility layer to make that work, because it uses temp directories in the file system. It looks like it is possible to bundle it with browserify. But the compat layer prevents it running. |
I see that is indeed in the converter spec. Should have read that more carefully. Looking into this. |
Prevents confusion with Node.js package index.
Causes an error when run on the bundled har-to-k6 package.
Well, tried to get browserify to run in the browser but I'm not having any luck. It does strange things with the file system. I've handled this by prebundling all possible compat layers. It adds a few megabytes. The code formatter, awkwardly, tries to load config from the file system. So I've had to disable it in the browser. Tried to generate in that format all the way through so it was redundant anyway. It should come out OK. If you do |
Shoot, I see it too in Chrome. Looking into that. |
Default behavior of the terser optimizer transforms ASCII escapes in regular expressions to raw Unicode characters. Chrome disapproves of the output. Disables Unicode characters in regular expressions and strings.
Pushed a fix to this. One of the optimizers was tweaking regular expressions in a way Chrome didn't like. Disabled that behavior. |
Yes it does work, sweet! 🎉
|
Strange, will look into it. |
I'm not sure there's a way to do this. Tried bundling some modules with different settings, but none of them get an importable module. Browserify seems to be targeting usage through the script tag. |
I managed to get rid of that error by adding
|
Updated that. |
Great, thank you! One of the use cases for this converter is to be used in a browser via a UI form drag and drop type of thing. So the idea is that a user should be able to compose a K6 script by click/drag and input. I think It would create a better UX if we could reduce the need for the compat file to a minimal. |
It would take some doing.
I could take these up as improvements under new issues. I see there are roadmap issues to add them to the engine grafana/k6#992 grafana/k6#991. Maybe I could evaluate the cost to do those. The converter tries hard to use builtin features wherever possible. Most of the time that formUrlEncode piece shouldn't be activated:
MimeBuilder usage does a similar thing of minimizing circumstances where it's loaded. |
Aight good to know, I'll add an issue that can be revisited once K6 supports those apis. |
Thank you for the review. |
Implements the
har-to-k6
converter, including API and CLI interfaces.CLI usage:
API usage:
Builds an optimized compatibility layer when needed.
Validates with what I hope are useful error messages.
validate()
exposed for separate use.There are some examples that can be converted in
example
.example/full.har
tries to exercise everything. It should run a successful live test against httpbin.