saucelabs / saucectl

A command line interface for the Sauce Labs platform.
Apache License 2.0
42 stars 16 forks source link

[proposal] Allow test runner specific configurations #76

Closed christian-bromann closed 3 years ago

christian-bromann commented 4 years ago

The problem

We have various types of configurations when running a test with saucectl. They can be categorized as:

The first two are already part of the current config specification. The last one isn't. Having framework specific configurations is particularly important for frameworks like WebdriverIO that define a configuration for running tests. However this can be also useful for already supported frameworks like Puppeteer or Playwright that run on Jest where the Jest configuration is currently hard coded into the runner images.

Proposal

After some initial discussions I propose to create a second configuration file that user can apply to their test runs, similar to the Kubernetes model, e.g.:

$ saucectl run -c ./.sauce/main.config.yml -c ./.sauce/framework.config.yml

The saucectl would then propagate the configurations to the runner where it will be consumed and applied. In order to distinguish the different types of configurations we would introduce a config property called Type that defines the configuration type. We already define this in our current configurations but ignore the value. The format of this property depends on the design we decide to chose.

Details

The configuration should be validated and send over to the runner image. There should be a fixed defined place for all configurations that allows the runner image to consume the configurations as files. I think Yaml is a format that can be read by all languages. Every framework has different kind of configurations. There are two ways how we can handle these:

Proposal A: one config that rules them all

One options is to allow the runner to define the format of the configurations based on the framework combination it offers (e.g. Jest + Puppeteer or TestNG + Selenium). I can also see there are even more setting variations depending on what the runner supports. This all can be then defined in a single Yaml file.

The general format should be as follows:

apiVersion: v1

# type of object: framework configurations
kind: FrameworkOptions

# maybe high level sauce configurations available in all runners
option1: "foo"
option2: "bar"

# a list of frameworks and their corresponding options
frameworks:
  # TestNG specific settings
  - TestNG:
      option1: "foo"
      option2: "bar"
  # Selenium specific settings
  - Selenium:
      option1: "foo"
      option2: "bar"

Advantages

Disadvantages

Proposal B: one config per framework

Another option could be to have a config file per framework. The structure of that would then look as follows:

jest.config.yaml:

kind: FrameworkOption
spec:
  testEnvironment: node
  reporters:
    - spec
    - junit

puppeteer.config.yaml:

kind: FrameworkOption
base: ./puppeteer.main.config.yaml
spec:
  headless: true
  defaultViewport:
    width: 800
    height: 600

Then run these configs with saucectl via:

$ saucectl run -c main.config.yaml -c jest.config.yaml -c puppeteer.config.yaml

Advantages

Disadvantages

Considerations

In the JavaScript ecosystem it is common that such configuration files are defined as executable JavaScript files, e.g. Webpack, that allows users to dynamically create a configuration object based on arbitrary conditions (e.g. environment variables). I can see this being also useful for this case particularly given that e.g. WebdriverIO users can define their config as a JS file but need to define it in yaml using saucectl. I would consider two options here:

dpgraham commented 4 years ago

I'm completely onboard with having framework specific configurations over general purpose configurations. I do think perhaps we ought to just have separate config files for separate frameworks instead of having multiple frameworks in one config.

e.g.)

If we had Cypress and WebDriverIO running side by side it could look like

# cypress-config.yml
kind: cypress
cypress:
  project: /path/to/cypress/project
  config:
    integrationsFolder: <...>

and

# wdio-config.yml
kind: webdriverio
...
webdriverio:
  conf: ./path/to/wdio.conf.js
christian-bromann commented 4 years ago

@dpgraham good point. I updated the proposal to incorporate this idea. Even though I feel a bit concerned about the fact that there could be a lot of configs I kind of tend towards proposal B. What do you think?

alexplischke commented 4 years ago

re. Proposal B:

too many configurations, users loose overview on what is defined where

I was thinking of having the ability to have the configs inside one file, which are then divided by the yaml document separator ---.

apiVersion: v1alpha
kind: Project
metadata:
  name: Feature XYZ
  tags:
    - e2e
    - release team
    - other tag
  build: Release $CI_COMMIT_SHORT_SHA
sauce:
  region: us-west-1
---
apiVersion: v1alpha
kind: Cypress
projectHome: /myproject
suites:
  - name: "saucy test"
    match: ".*.(spec|test).js$"
image:
  base: saucelabs/stt-cypress-mocha-node
  version: latest

This allows the user to still retain the ability to have a quick overview of the settings.

PS: Don't pay much attention to how I separated the attributes between the two configs. Its purpose is to illustrate the separation.

christian-bromann commented 4 years ago

@alexplischke sounds reasonable to me! With which idea should we move forward?

dpgraham commented 4 years ago

Cypress Use-Case (will be applicable to Jest, WDIO, etc...)

Proposal A (command line)

The cypress.config.yml looks like this:

apiVersion: v1
kind: cypress
spec:
  projectHome: /bobs-project-home
  options: # These can be anything from https://docs.cypress.io/guides/references/configuration.html#Options
    viewportWidth: 660
    viewportHeight: 1000

The saucectl commands look like this

npx saucectl --config cypress.config.yml
npx saucectl --config cypress.config.yml --options.viewportWidth 500 --options.viewportHeight 1000
npx saucectl --config cypress.config.yml --options.viewportWidth 1200 --options.viewportHeight 1500

Proposal B (config file)

apiVersion: v1
kind: cypress
spec:
  projectHome: /bobs-project-home
suites:
  - name: medium-screen
    options:
      viewportWidth: 660
      viewportHeight: 1000
  - name: small-screen
    options:
      viewportWidth: 500
      viewportHeight: 1000
  - name: large-screen
     options:
       viewportWidth: 1200
       viewportHeight: 1500
npx saucectl run --config cypress.config.yml

I lean towards Proposal A... I think we ought to give as little friction as possible between running Cypress locally/in CI as it is to run through saucectl

With proposal A, all the user needs to do is:

  1. create the Cypress config file using interactive prompts
  2. translate the cypress command (cypress run --viewportWidth 100 --viewportHeight 200) to the saucectl command (saucectl run --config cypress.config.yml --options.viewportWidth 100 --options.viewportHeight 200)
dpgraham commented 4 years ago

^ this is similar to what Robinhood (a la bootstraponline) was asking for:

Hey all, tried looking into helping robinhood, got into a bit of a pickle since i'm not really familiar with how DevX is running Playwright. Here is their Q: I'd like my playwright tests to run when I type saucectl run. When I type saucectl new and select playwright, I expect the template to be using the playwright test runner. There also needs to be a way to pass values to the playwright test runner. Instead of typing `npx test-runner --browser-name=chromium` I could type `saucectl run --browser-name=chromium` or similar.
12:51
I think what they want is for the tests written using this framework (which need to be appended with the npx command) to work with the saucectl command
christian-bromann commented 4 years ago
  • How does Bob accomplish this?

@dpgraham I would also say Proposal A makes more sense. We should not bring any parallisation logic into the framework options that just makes it too complex. I would that you would always want to use the same configurations across tests run in parallel otherwise how can you ensure that they run in the same conditions.

I also noted that we have a different interpretation for kind. In my proposal I would have it as kind: FrameworkOption whereas you both put the framework name into it kind: cypress. Given that kind describes the format of the configuration spec I would keep a consistent name for it, e.g. FrameworkOption. While it is indeed important to reference the framework these options apply to I would rather define it in an extra field:

jest.config.yaml:

kind: FrameworkOption
framework: jest
spec:
  testEnvironment: node
  reporters:
    - spec
    - junit

puppeteer.config.yaml:

kind: FrameworkOption
base: ./puppeteer.main.config.yaml // framework is defined in base
spec:
  headless: true
  defaultViewport:
    width: 800
    height: 600

WDYT?

alexplischke commented 4 years ago

To capture what I said earlier in the standup: If it's just the framework itself that is interested in reading those settings, then it's fine imho to use your approach:

kind: FrameworkOption
framework: jest
spec:
  testEnvironment: node
  reporters:
    - spec
    - junit
alexplischke commented 3 years ago

Closing ticket. We went with the following framework native config approach:

apiVersion: v1alpha
kind: cypress
sauce:
  region: us-west-1
  metadata:
    name: Testing Cypress Support
    tags:
      - e2e
      - release team
      - other tag
    build: Release $CI_COMMIT_SHORT_SHA
docker:
  image:
    name: saucelabs/stt-cypress-mocha-node
    tag: v0.2.0
cypress:
  configFile: "tests/e2e/cypress.json"  # We determine related files based on the location of the config file.
suites:
  - name: "saucy test"
    browser: "chrome"
    browserVersion: "82" # Irrelevant when using docker.
    config:
      env:
        hello: world
      testFiles: [ "**/*.*" ] # Cypress native glob support.

Cypress is the first framework for which we support this new approach. Similar support for other frameworks will follow.