-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add core to inject k6 object into window #1007
Conversation
3cf3de1
to
ef03900
Compare
In some cases we want to pass on information to other apps to allow them to infer who or what is performing requests against a website. For now we're exposing an empty object, but we hope to extend the object with meta data that will be useful for other apps that have been instrumented on the website.
This test will evaluate and return the value. It should be an empty object and not null.
ef03900
to
6f5222b
Compare
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.
Looks good to me overall, and I'm excited about this feature! 😊
I do have a few points for us to chat about. Also, it might be helpful to include some comments in the code to clarify the role of k6ObjScript
.
Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>
We're renaming this since this file will embed scripts other than web vital scripts.
To be consistent with how we work with other injected scripts, the k6 object is being moved to its own js file. This also helps us avoid working with globals. It's worth noting that the k6 object will need to be mutable at some stage to add test specific unique metadata in the future. Resolves: #1007 (comment)
This commit now works with the new k6 object file instead of the global k6 object file. Resolves: #1007 (comment)
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.
Nice job, thanks!
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.
LGTM 👏
To be consistent with how we work with other injected scripts, the k6 object is being moved to its own js file. This also helps us avoid working with globals. It's worth noting that the k6 object will need to be mutable at some stage to add test specific unique metadata in the future. Resolves: #1007 (comment)
After having a discussion, we decided to make this parallel. See the related discussion here: #1007 (comment)
After having a discussion, we decided to make this parallel. See the related discussion here: #1007 (comment)
After having a discussion, we decided to make this parallel. See the related discussion here: #1007 (comment)
After having a discussion, we decided to make this parallel. See the related discussion here: #1007 (comment)
After having a discussion, we decided to make this parallel. See the related discussion here: #1007 (comment)
What?
The change injects
window.k6 = {};
into all browser contexts and subsequentpage
s that are created.Why?
In some cases we want to pass information to other apps to allow them to infer who is performing requests against a website. For now we're exposing an empty object, but we hope to extend the object with meta data that will be useful for other apps that have been instrumented on the website.
Test
To test this change you can run the following k6 script, which will give you enough time to go into the chrome dev tools console and type in
window.k6
which should return an empty object ({}
).Checklist
Closes: #1017