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 experimental JS modules #2630

Merged
merged 1 commit into from
Aug 16, 2022
Merged

Conversation

oleiade
Copy link
Member

@oleiade oleiade commented Aug 2, 2022

Rationale

This PR makes xk6-redis, xk6-timers and xk6-websockets extensions importable under the k6/experimental import path. Their code is not imported in k6's codebase, but instead added as dependencies, which allows for more flexibility during their integration and improvement process moving forward.

Review

The code of each module themselves was already reviewed in their source extension's context. We'd be more interested in getting feedback on the integration. Modules are importable as js/modules/k6/experimental in k6 scripts, but their code remains in their respective k6 extensions, who have been especially structured to permit this import.

Don't let the size of the PR scare you, most of it is "just" vendor anyway.

The significant files are:

  • js/initcontext.go: exposes the modules under k6/experimental
  • samples/experimental/*.go: exposes examples of the experimental
    module in action
  • samples/docker-compose.yml: allows spinning up a stack for testing
    the Redis module

Let me know what you think 🙇🏻

@oleiade oleiade added the feature label Aug 2, 2022
@oleiade oleiade added this to the v0.40.0 milestone Aug 2, 2022
@oleiade oleiade self-assigned this Aug 2, 2022
@github-actions github-actions bot requested review from mstoykov and na-- August 2, 2022 12:28
@mstoykov
Copy link
Contributor

mstoykov commented Aug 2, 2022

The error you are getting in the tests is due to 1b0e24e . You should probably bump the k6 version in the extension and fix it there for now.

@oleiade
Copy link
Member Author

oleiade commented Aug 2, 2022

Checklist:

  • switch to include method pointed out by mihail
  • fix test

@oleiade oleiade force-pushed the feature/js/experimental_redis branch 2 times, most recently from 844cf7b to 7350f8c Compare August 3, 2022 08:18
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general, but

  1. Can you add the websockets and timers as well just so we don't open multiple PRs (I am lazy)
  2. I think the experimental's README needs an update

@oleiade oleiade force-pushed the feature/js/experimental_redis branch 2 times, most recently from 0b30c35 to 7b82bcb Compare August 15, 2022 10:09
@oleiade oleiade force-pushed the feature/js/experimental_redis branch 2 times, most recently from c29fe73 to 2cddbe2 Compare August 15, 2022 12:02
@oleiade oleiade changed the title Introduce experimental redis JS module Introduce experimental JS modules Aug 15, 2022
@oleiade oleiade requested a review from mstoykov August 15, 2022 12:08
@oleiade oleiade force-pushed the feature/js/experimental_redis branch from 2cddbe2 to bdeca6a Compare August 15, 2022 12:12
@na-- na-- requested a review from olegbespalov August 16, 2022 08:11
na--
na-- previously approved these changes Aug 16, 2022
@oleiade oleiade force-pushed the feature/js/experimental_redis branch 3 times, most recently from ffaf255 to 097b560 Compare August 16, 2022 08:48
Comment on lines +47 to +50
github.com/cespare/xxhash/v2 v2.1.2 // indirect
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
github.com/go-redis/redis/v8 v8.11.5 // indirect
github.com/mstoykov/k6-taskqueue-lib v0.1.0 // indirect
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be in the bottom block 🤔

mstoykov
mstoykov previously approved these changes Aug 16, 2022
olegbespalov
olegbespalov previously approved these changes Aug 16, 2022
@oleiade oleiade requested review from mstoykov and na-- August 16, 2022 09:04
mstoykov
mstoykov previously approved these changes Aug 16, 2022
na--
na-- previously approved these changes Aug 16, 2022
This commit imports the latest state of xk6-redis, xk6-websockets, and
xk6-timers into k6's core as experimental modules importable under the
"k6/experimental" import path.

Specifically regarding the redis module: with it comes a single direct
dependency, the `redis/v8` library, and two indirect dependencies
implied by the former `xxhash` and `go-rendezvous`.

This commit's essential files are:
* `js/initcontext.go`: exposes the modules under `k6/experimental`
* `samples/experimental/*.go`: exposes examples of the experimental
module in action
* `samples/docker-compose.yml`: allows to spin up a stack for testing
the redis module

The rest of the files are essentially go module and vendoring plumbing.
@oleiade oleiade dismissed stale reviews from na--, mstoykov, and olegbespalov via da648a9 August 16, 2022 13:13
@oleiade oleiade force-pushed the feature/js/experimental_redis branch from a997947 to da648a9 Compare August 16, 2022 13:13
@oleiade
Copy link
Member Author

oleiade commented Aug 16, 2022

It's green again 🪴 🌱

@oleiade oleiade merged commit d191029 into master Aug 16, 2022
@oleiade oleiade deleted the feature/js/experimental_redis branch August 16, 2022 14:02
@codebien codebien mentioned this pull request Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants