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

feat(test-runner): minimal proof-of-concept scheduler API #16698

Closed
wants to merge 1 commit into from

Conversation

rwoll
Copy link
Member

@rwoll rwoll commented Aug 20, 2022

Proof-of-concept for #14895 (comment).

About

This PR introduces a proof-of-concept, minimal user-facing scheduling API that attempts to address a variety of longstanding issues and feature requests.

At this point, the implementation details should be ignored (although look if you want a puzzle 😄 ). However, the PR serves to allow us to concretely discuss a proposed solution, the general test strategy (which will apply regardless of the solution/API), articulate pros/cons/problems, etc. (The implementation is currently enough so that we can play around with the proposed public API to get a feel for it in the context of the use cases it attempts to solve.)

API Overview

This introduces a projectSchedule?: string[][] configuration option at the top-level, which can be used in combination with the already existing projects configuration option to guarantee which projects run in relation to others.

Using it looks something like:

      projects: [
        { name: 'My Setup', testDir: './setup',  },
        { name: 'Desktop', testDir: './desktop', use: devices['Desktop Chrome'] },
        { name: 'Mobile', testDir: './mobile', use: devices['iPhone 11'] },
        { name: 'My Teardown', testDir: './teardown', },
      ],
      projectSchedule: [
        ['My Setup'],
        ['Desktop', 'Mobile'],
        ['My Teardown']
      ],
    },

In this case, this simply means: run all tests in the My Setup project, when that's done run all the tests in Desktop and Mobile project in parallel, then run My Teardown project.

Issues Addressed

  1. User wants to run global setup/teardown code and have a trace in the HTML Report
  2. Trace, Retry, Device, etc. Settings from Config Should Apply to Global Setup/Teardown
  3. Different Global Setup Per-Project
  4. Allow Fixtures to Be Used in Global Setup
  5. How to Use Create and Use Storage State for Tests in Global Setup
  6. Run Files/Projects in Certain Order AND Include in One HTML Report

From memory, there are other issues (and Slack conversations) that fall into some of the buckets above, too.

Pros

  1. minimal, but effective(?) API: less is more. It's much easier to explain and program than some fancy DAG-based API
  2. everything's just a test! we get config, tracing, retries, fixtures, etc. for free — no more trying to figure out how to use globalSetup/globalTeardown

Cons/Limitations/Questions

When using projectSchedule:

  • cannot use shard option: if you're using the schedule, you can quasi-shard with the schedule itself
  • project names must be unique

In the above example, if I want to run Desktop project, but not Mobile, how do we express this? Presumably we need to run My Setup and My Teardown, too.

We're not exposing a full-featured DAG, so dependency expression is limited (I think for the better, for now — although we can do some really neat scheduling optimizations in the future with 1 timing run + the DAG).

We don't guarantee projects run on the same worker/process, so you can't, for instance, start a webserver in memory in My Setup and kill it in My Teardown. Although, in this case you can just use the webServer config option, or write a PID in your setup, then read and kill that PID in your teardown.

@rwoll rwoll force-pushed the feat/ordered-projects branch from 46041e4 to 597c7f6 Compare August 20, 2022 00:18
@rwoll rwoll force-pushed the feat/ordered-projects branch from 597c7f6 to cc7bbab Compare August 20, 2022 00:21
@rwoll rwoll requested a review from dgozman August 20, 2022 00:38
@rwoll rwoll changed the title [WIP] feat(test-runner): minimal proof-of-concept scheduler API feat(test-runner): minimal proof-of-concept scheduler API Aug 20, 2022
@rwoll
Copy link
Member Author

rwoll commented Aug 22, 2022

Initial feedback from @dgozman (my responses inline):

  1. Is seems equivalent to introducing priority field on the project - is that right?

Yes, it's functionally equivalent. However, I think if we omitted projectSchedule and just did priority on project, you'd force the user to build up schedule array you see in projectSchedule anyway. Priority of an individual project is not helpful, since it really only matters in relation to the other project priorities, so you end up building this in your head.

  1. We do not address "global setup per project"

We do, (although it's not necessarily pretty). It would look something like:

      projects: [
        { name: 'Desktop Setup', testDir: './desktop/setup', use: devices['Desktop Chrome']  },
        { name: 'Desktop', testDir: './desktop/tests', use: devices['Desktop Chrome'] },
        { name: 'Mobile Setup', testDir: './mobile/setup', use: devices['iPhone 11'] },
        { name: 'Mobile', testDir: './mobile/tests', use: devices['iPhone 11'] },
      ],
      projectSchedule: [
        ['Desktop Setup', 'Mobile Setup'],
        ['Desktop', 'Mobile'],
      ],
  1. --project and --shard kinda mess things up

--shard I need to think more on (and intentionally punted per the limitations section), but I think the --project issue is resolvable by using a project-level-DAG instead of a test level-DAG.

i.e. scrap the projectSchedule list. And instead allow a each project to specify dependsOn: […]:

      projects: [
        { name: 'Desktop Setup', testDir: './desktop/setup', use: devices['Desktop Chrome']  },
        { name: 'Desktop', testDir: './desktop/tests', use: devices['Desktop Chrome'], dependsOn: ['Desktop Setup'] },
        { name: 'Mobile Setup', testDir: './mobile/setup', use: devices['iPhone 11'] },
        { name: 'Mobile', testDir: './mobile/tests', use: devices['iPhone 11'], dependsOn: ['Mobile Setup'] },
      ]

So if a user does, npx playwright test --project=Mobile it would run any dependencies, too (e.g. Mobile Setup project) in this case.

  1. Great job on the detailed description and finding related issues!

😄

@rwoll
Copy link
Member Author

rwoll commented Aug 30, 2022

Closing for now — after much offline discussion, we have a new API proposal, so we will re-open with that implementation.
.

@rwoll rwoll closed this Aug 30, 2022
@carlpaten
Copy link

@rwoll is the new API proposal publicly available?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants