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

Update context, newContext and newPage description for browser module #1242

Merged

Conversation

ankur22
Copy link
Contributor

@ankur22 ankur22 commented Jun 30, 2023

Due to the change in grafana/xk6-browser#929 and grafana/xk6-browser#945, we need to update the following to reflect this new behaviour where a browser can only hold a single browserContext and if a user wishes to create a new browserContext then they can only close the existing one and create a new one.

  • Change browser.contexts to browser.context to retrieve the current browserContext.
  • Change the description in browser.newPage to help user understand how they can work with more than one browserContext sequentially.
    • Change the description in browser.newContext to help user understand how they can work with more than one browserContext sequentially.

@ankur22 ankur22 requested review from ka3de and mdcruz June 30, 2023 13:58
@ankur22 ankur22 self-assigned this Jun 30, 2023
@ankur22 ankur22 changed the base branch from main to update/browser-cloud-updates June 30, 2023 13:58
@ankur22 ankur22 force-pushed the refactor/925-refactor-contexts branch 2 times, most recently from ebd370d to 6d1f78a Compare June 30, 2023 14:10
@github-actions
Copy link
Contributor

There's a version of the docs published here:

https://mdr-ci.staging.k6.io/docs/refs/pull/1242/merge

It will be deleted automatically in 30 days.

ankur22 added 4 commits July 4, 2023 16:38
We've made a change which means that there is now a 1-to-1 mapping
between browser and a browserContext. This means that there can only
ever be one browserContext open at any given point per browser. So
the contexts method on browser now only needs to return the current
browserContext.
The discription on browser.newContext needs to reflect the new change
where only one browserContext can be open at any given time per
browser. We need to notify the user how they can work with this new
behaviour.
The description on browser.newPage needs to reflect the new change
where only one browserContext can be open at any given time per
browser. We need to notify the user how they can work with this new
behaviour.
@ankur22 ankur22 force-pushed the refactor/925-refactor-contexts branch from 6d1f78a to 4ff194e Compare July 4, 2023 15:39
Copy link
Contributor

@ka3de ka3de left a comment

Choose a reason for hiding this comment

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

Overall LGTM 👏 Just made a small comment.

… browser/context.md

Co-authored-by: ka3de <daniel.jimenez@grafana.com>
@ankur22 ankur22 merged commit 77faaa8 into update/browser-cloud-updates Jul 7, 2023
@ankur22 ankur22 deleted the refactor/925-refactor-contexts branch July 7, 2023 09:13
@inancgumus inancgumus added the Area: browser The browser module label Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: browser The browser module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants