xk6-browser should be importable as a k6 module #667
Conversation
Moves the module registration logic to a new pkg called browser.
inancgumus
force-pushed
the
add/662-importable-browser
branch
from
1d59256
to
22956cf
Compare
inancgumus
force-pushed
the
add/662-importable-browser
branch
from
22956cf
to
35f8dac
Compare
This change made to be consistent. As it used like so in other modules.
inancgumus
force-pushed
the
add/662-importable-browser
branch
from
35f8dac
to
4a01306
Compare
|
|
||
| // TestModuleNew tests registering the module. | ||
| // It doesn't test the module's remaining functionality as it is | ||
| // already tested in the tests/ integration tests. |
There was a problem hiding this comment.
Shall the tests then just import browser and test it ?
There was a problem hiding this comment.
preferably not in this PR but later ;)
There was a problem hiding this comment.
That makes sense, thanks :) However, I liked keeping this simple test next to the module registration. So that it would be easier to make changes to both code and tests when needed (instead of jumping to the other parts of the code base (tests/).
| ) | ||
|
|
||
| func init() { | ||
| if _, ok := os.LookupEnv("K6_BROWSER_PPROF"); ok { |
There was a problem hiding this comment.
I guess we should remember that this won't work with core k6.
There was a problem hiding this comment.
That's fine tbh.
Out of curiosity, why wouldn't the init function run when k6 imports the module? I thought that all init functions had to to run (which is an unwanted side affect sometimes).
There was a problem hiding this comment.
you import a package.
And we are not going to import this package as that will also register the extension as k6/x/browser - the init below this one.
Which is like the whole point of this PR ;)
There was a problem hiding this comment.
We use this code in our own instances for testing, so that won't be a problem. Thanks for letting us know, though 👍
Closes #662