-
Notifications
You must be signed in to change notification settings - Fork 42
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
Use k6 events to handle browser lifecycle #944
Conversation
07702be
to
a8145bc
Compare
f10d8e7
to
334c8b3
Compare
In this way we reduce contention on sync.Map and avoid repeated string concatenation calls when generating the browser registry index, which was built based on VUID-scenario-iteration. Instead, tiying the browser registry scope to a single VU, we can use just the iteration as index for the registry.
This will allow for more flexibility on browser operations, for example during cleanup, on which all browser entries in the registry must be closed and deleted.
8986bef
to
ebaae44
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.
Thanks for pushing this through and working with the k6 changes to get it to this final stage 👏
I think it mostly makes sense. I've left some questions that would help me understand the behaviour of the event system a bit more. Once those have been clarified though, i think we're good to go 🙌
This commit modifies the browser registry to subscribe to k6 'IterStart' and 'IterEnd' events in order to initialize and close the browser instances when receiving these events.
Adds a new event handler for the 'Exit' event, which is guaranteed to be fired before k6 execution exits, even in case of panic. Therefor this event is used to ensure all browser instances held at the VU registries are closed before finishing execution.
Instead, now browsers are only initialized and indexed in the registry as a reaction to 'IterStart' events. Therefor we can remove the getOrInitBrowser method from the mapping layer, and only call vu.getBrowser method in order to retrieve the previously initialized browser for the iteration.
As now it is only called from within the browser registry, and not from the mapping layer.
Now that browsers are not initialized or closed from the mapping layer, but as a reaction to iteration events from browser registry, we need helper functions for tests that require testing through the mapping layer so the browser for the executed iteration during the test can be previously initialized, and closed at the end of the iteration if necessary.
Use the vu.StartIteration helper in order to pre initialize the browser required for the iteration executed during the test, so once the JS and mapping layer code is executed, the browser instance can be correctly retrieved.
Add a higher level helper method to VU in order to retrieve the current VU iteration's browser instance. This improves the repetitive calls to getBrowser() method passing the current iteration as input from the mapping layer.
This will match our own go.mod version Also, this test was failing due to the usage of URL.JoinPath method in k6, which is only available since go v1.19.
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 🚀 Thanks for taking the time to iterate with k6 core on this 🙌
Description of changes
This PR integrates k6 browser, and more specifically our implementation of the browser registry, with the events generated from k6 (see grafana/k6#3112) in order to handle the browser lifecycle based on these events. Specifically:
IterStart
event.IterEnd
event.Exit
event (which is the event guaranteed to be fired even on case of unexpected error).Closes #926.
Checklist