Modules fixes #2234
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,26 +40,26 @@ type ( | |
|
|
||
| // ModuleInstance represents an instance of the execution module. | ||
| ModuleInstance struct { | ||
| vu modules.VU | ||
| obj *goja.Object | ||
| } | ||
| ) | ||
|
|
||
| var ( | ||
| _ modules.Module = &RootModule{} | ||
| _ modules.Instance = &ModuleInstance{} | ||
| ) | ||
|
|
||
| // New returns a pointer to a new RootModule instance. | ||
| func New() *RootModule { | ||
| return &RootModule{} | ||
| } | ||
|
Comment on lines
54
to
56
There was a problem hiding this comment. What I'm going to say would probably be involved and may not be an easy task. And, I'm not deeply knowledgeable about the system and our goals, so this can be a fruitless suggestion, and it might not even work 🤷 Why don't we register in an extension without giving the k6 an instance of the module, like this: func init() {
k6modules.Register("k6/x/browser")
}With this, we won't have to stutter in tests, and we can more natural type something like this: New(mii)Instead Of: New().NewModuleInstance(mii)So the interface would look like this: type IsModuleV2 interface {
New(InstanceCore) Instance
}In an extension we can say: type RootModule struct {
modules.InstanceCore
}
var _ modules.IsModuleV2 = &RootModule{}
func New(m modules.InstanceCore) *RootModule {
return &RootModule{InstanceCore: m}
}Instead of: type (
RootModule struct{}
ModuleInstance struct {
modules.InstanceCore
}
)
var (
_ modules.IsModuleV2 = &RootModule{}
_ modules.Instance = &ModuleInstance{}
)
func (*RootModule) NewModuleInstance(m modules.InstanceCore) modules.Instance {
return &ModuleInstance{InstanceCore: m}
}
func New() *RootModule {
return &RootModule{}
}Of course, with this, we need to create a struct (RootModule) of a module ourselves. But then again, we can standardize it like RootModule and instantiate it using reflection. There was a problem hiding this comment. The biggest (and most obvious problem) is that this all si also used for extensions. And while for internal we can have it listed the way it currently works (and likely will still need to work) is that extensions register themselves in a init function and add the Hope this explains the problem enough, but I would recommend reading the provided issue. |
||
|
|
||
| // NewModuleInstance implements the modules.Module interface to return | ||
| // a new instance for each VU. | ||
| func (*RootModule) NewModuleInstance(vu modules.VU) modules.Instance { | ||
| mi := &ModuleInstance{vu: vu} | ||
| rt := vu.Runtime() | ||
| o := rt.NewObject() | ||
| defProp := func(name string, newInfo func() (*goja.Object, error)) { | ||
| err := o.DefineAccessorProperty(name, rt.ToValue(func() goja.Value { | ||
|
|
@@ -82,25 +82,25 @@ func (*RootModule) NewModuleInstance(m modules.InstanceCore) modules.Instance { | |
| return mi | ||
| } | ||
|
|
||
| // Exports returns the exports of the execution module. | ||
| func (mi *ModuleInstance) Exports() modules.Exports { | ||
| return modules.Exports{Default: mi.obj} | ||
| } | ||
|
|
||
| // newScenarioInfo returns a goja.Object with property accessors to retrieve | ||
| // information about the scenario the current VU is running in. | ||
| func (mi *ModuleInstance) newScenarioInfo() (*goja.Object, error) { | ||
| ctx := mi.vu.Context() | ||
| rt := common.GetRuntime(ctx) | ||
| vuState := mi.vu.State() | ||
| if vuState == nil { | ||
| return nil, errors.New("getting scenario information in the init context is not supported") | ||
| } | ||
| if rt == nil { | ||
| return nil, errors.New("goja runtime is nil in context") | ||
| } | ||
| getScenarioState := func() *lib.ScenarioState { | ||
| ss := lib.GetScenarioState(mi.vu.Context()) | ||
| if ss == nil { | ||
| common.Throw(rt, errors.New("getting scenario information in the init context is not supported")) | ||
| } | ||
|
|
@@ -140,7 +140,7 @@ func (mi *ModuleInstance) newScenarioInfo() (*goja.Object, error) { | |
| // newInstanceInfo returns a goja.Object with property accessors to retrieve | ||
| // information about the local instance stats. | ||
| func (mi *ModuleInstance) newInstanceInfo() (*goja.Object, error) { | ||
| ctx := mi.vu.Context() | ||
| es := lib.GetExecutionState(ctx) | ||
| if es == nil { | ||
| return nil, errors.New("getting instance information in the init context is not supported") | ||
|
|
@@ -175,7 +175,7 @@ func (mi *ModuleInstance) newInstanceInfo() (*goja.Object, error) { | |
| // newVUInfo returns a goja.Object with property accessors to retrieve | ||
| // information about the currently executing VU. | ||
| func (mi *ModuleInstance) newVUInfo() (*goja.Object, error) { | ||
| ctx := mi.vu.Context() | ||
| vuState := lib.GetState(ctx) | ||
| if vuState == nil { | ||
| return nil, errors.New("getting VU information in the init context is not supported") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just use
moduleVUinstead ofmoduleVUImpl?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some hopes I will be working on the TODO above this cycle, and it will just get completely removed