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

Expose a new browser specific option in scenarios #3022

Closed
ankur22 opened this issue Apr 18, 2023 · 5 comments · Fixed by #3036
Closed

Expose a new browser specific option in scenarios #3022

ankur22 opened this issue Apr 18, 2023 · 5 comments · Fixed by #3036
Assignees
Labels

Comments

@ankur22
Copy link
Contributor

ankur22 commented Apr 18, 2023

Feature Description

How we currently work with the browserType and browser

With the k6-browser module we currently have to be very explicit in how we define and create the browser object (which starts and connects to the browser instance), which we now feel isn't a great DX. We also allow any number of browsers to be launched, which we think is a feature that isn't particularly useful. Each iteration will start and stop a browser process, which we think is wasteful since running browser instances is an expensive process.

In general we want to be able to abstract away the lifecycle of running the browser, and at the same time reduce the number of browser instances a test run needs to work with.

Suggested Solution (optional)

We think the solution is for the k6-browser module (or an external process) to take ownership of the browser instances lifecycle depending on a heuristic that it sees fit for the test requirements. For the module/process to determine how many browser instances it requires, we want to leverage scenarios which will help us determine how many vus a test run will need. The k6-browser module will restrict the number of browserContexts to one per vu per iteration. So in the following example where we have defined one scenario, we know that the test will require 10 vus, and the module/process can work with this number to start up a suitable number of browser instances that can be shared between all the iterations.

export const options = {
  scenarios: {
    ui: {
      executor: 'constant-vus',
      exec: 'ui',
      vus: 10,
      duration: '10s',
      options: {
        browser: {
          type: 'chromium'
        },
      },
    },
  },
};

export async function ui() {
  // test code here
}

To clarify, the requested change is specifically to add this:

      options: {
        browser: {
          type: 'chromium'
        },
      },

Note that the validation of the properties should be done by the k6-browser module.

Already existing or connected issues / PRs (optional)

No response

@imiric
Copy link
Contributor

imiric commented Apr 19, 2023

@ankur22 I have a few questions about this feature.

  1. The k6-browser module will restrict the number of browserContexts to one per vu per iteration.

    This doesn't seem different from the current implementation if the script launches and closes one browser instance in the default function. I was under the impression that one k6 VU would map exactly to one BrowserContext, regardless of the number of iterations. I.e. k6-browser would launch a browser (context) when it's initialized (considering there's only one instance of the browser module per VU to begin with), and then this browser context would remain active for the lifetime of that VU, and would be reused by every iteration that VU executes.

    The above "one BrowserContext per VU per iteration" sounds confusing to me, so please let me know if my assumption is correct. Your later note that "browser instances [...] can be shared between all the iterations" hints towards this, but I wanted to confirm we're on the same page.

  2. The options property under the scenario sounds superfluous to me, considering these are all options that control test execution.

    WDYT about removing that layer in the structure, and placing the browser property directly under the scenario? I.e.:

    export const options = {
      scenarios: {
        ui: {
          executor: 'constant-vus',
          exec: 'ui',
          vus: 10,
          duration: '10s',
          browser: {
            type: 'chromium'
          },
        },
      },
    };

    I haven't followed previous discussions about this, so there might be a reason we wanted to group generic "scenario options" together. Maybe so that the actual JSON map in k6 core could have json.RawMessage values, in order to avoid k6 core being aware of the browser module at all, which I've heard mentioned before.

    Is this the reason for the options property? If so, I'll reiterate my position that the browser module is a first-class k6 component, and drawing this boundary doesn't make much sense. But I'll accept it if it has already been discussed and decided as the way it will be implemented.

    It would be good to mention such technical details in the "Suggested solution" section, and link to the relevant issue / document / thread where this was discussed.

  3. How will k6-browser read these options? Will you get them from VU.State.Options, from the goja runtime, or expect them to be exposed in some other way? This is part of a broader discussion about how JS extensions can be configured, but since k6-browser will eventually become part of k6 core, it would be good to not have to change this once the extension is merged.

    Considering VU.State.Options will probably be removed, it might not be wise to rely on it. But then again, it's the easiest way to do this right now, so you might not have a choice.

imiric pushed a commit that referenced this issue Apr 26, 2023
This is a quick and possibly naive change to support browser options per
scenario (#3022).

It doesn't try to enable generic options for any JS module as #3000 did,
but it seems Ned abandoned this idea. We can open this discussion again
once #883 is resolved.
imiric pushed a commit that referenced this issue Apr 26, 2023
This is a quick and possibly naive change to support browser options per
scenario (#3022).

It doesn't try to enable generic options for any JS module as #3000 did,
but it seems Ned abandoned this idea. We can open this discussion again
once #883 is resolved.
@ka3de
Copy link
Contributor

ka3de commented Apr 26, 2023

Hey @imiric !

I will try to answer your questions:

I was under the impression that one k6 VU would map exactly to one BrowserContext, regardless of the number of iterations. I.e. k6-browser would launch a browser (context) when it's initialized (considering there's only one instance of the browser module per VU to begin with), and then this browser context would remain active for the lifetime of that VU, and would be reused by every iteration that VU executes.

Our initial approach is that the lifetime of the BrowserContext will span only throughout the iteration. So we will create a new BrowserContext each time a VU iteration starts, and close it when this one finishes. In that way we ensure there is "isolation" also between iterations for the same VU.
Does this make sense?

The options property under the scenario sounds superfluous to me, considering these are all options that control test execution.
WDYT about removing that layer in the structure, and placing the browser property directly under the scenario?

To be honest I lack some context to really give an opinion here. For the case of k6 browser I believe it wont' change much, but I think this "intermediate" options object (which I think at some point was defined as params instead, to not repeat "options" naming inside the "global options") was proposed by @na-- so in the future other extension options could be defined there also.

What is your take on @imiric 's proposal @na-- ?

How will k6-browser read these options? Will you get them from VU.State.Options, from the goja runtime, or expect them to be exposed in some other way? This is part of a broader discussion about how JS extensions can be configured, but since k6-browser will eventually become part of k6 core, it would be good to not have to change this once the extension is merged.

Considering VU.State.Options will probably be removed, it might not be wise to rely on it. But then again, it's the easiest way to do this right now, so you might not have a choice.

Our initial idea was to use VU.State.Options as you mention (see grafana/xk6-browser#858). But we can explore other options if you think there are better alternatives right now or for the future.

@imiric
Copy link
Contributor

imiric commented Apr 26, 2023

Thanks for answering @ka3de! 🙇

I already discussed this last week with @ankur22 on Slack, so we're aligned on the way forward. Sorry for not updating this issue after the discussion.

See #3036 for a preliminary change for this. I decided to keep the nested options instead of params, as it's clearer, even with the repeated options.

@ka3de
Copy link
Contributor

ka3de commented Apr 26, 2023

Ok! I was not aware 😅 Saw the commit but was not sure if still these questions required clarification. Perfect then! 👍

@imiric imiric linked a pull request Apr 26, 2023 that will close this issue
@ankur22
Copy link
Contributor Author

ankur22 commented Apr 27, 2023

Thanks for answering @ka3de. I was meant to post the answers after the discussion.

imiric pushed a commit that referenced this issue Apr 28, 2023
This is a quick and possibly naive change to support browser options per
scenario (#3022).

It doesn't try to enable generic options for any JS module as #3000 did,
but it seems Ned abandoned this idea. We can open this discussion again
once #883 is resolved.
imiric pushed a commit that referenced this issue Apr 28, 2023
This is a quick and possibly naive change to support browser options per
scenario (#3022).

It doesn't try to enable generic options for any JS module as #3000 did,
but it seems Ned abandoned this idea. We can open this discussion again
once #883 is resolved.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants