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.

Sign up for GitHub

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

Jump to bottom

Split plan to multiple plans by --max tests #2907

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Split plan to multiple plans by --max tests #2907

wants to merge 5 commits into from

Conversation

happz
Copy link
Collaborator

@happz happz commented May 5, 2024

Pull Request Checklist

  • implement the feature
  • write the documentation
  • extend the test coverage
  • update the specification
  • adjust plugin docstring
  • modify the json schema
  • mention the version
  • include a release note

@happz
Copy link
Collaborator Author

happz commented May 5, 2024

Splitting is easy. Running them will be hard.

$ hatch run dev:tmt -c trigger=commit -vv run discover plan -n '^/plans/features/core$'
/var/tmp/tmt/run-006
Found 1 plan.

/plans/features/core
summary: Verify core functionality
    discover
        how: fmf
        directory: /home/happz/git/tmt
        hash: 78742f93
        filter: tier: 0, 1 & tag:-provision-only
        summary: 41 tests selected
            /tests/backward-compatibility
            /tests/core/adjust
            /tests/core/context
            /tests/core/debug
            /tests/core/docs
            /tests/core/dry
            /tests/core/enabled
            /tests/core/env
            /tests/core/environment-file
            /tests/core/error
            /tests/core/escaping
            /tests/core/feeling-safe
            /tests/core/fmf-id
            /tests/core/force
            /tests/core/link
            /tests/core/logging
            /tests/core/ls
            /tests/core/order
            /tests/core/path
            /tests/core/phases
            /tests/core/plan-env-file
            /tests/core/smoke
            /tests/core/spaces
            /tests/core/web-link
            /tests/init/base
            /tests/init/full
            /tests/init/git
            /tests/init/mini
            /tests/init/nested
            /tests/lint/all
            /tests/lint/plan/explicit-root
            /tests/lint/plan/implicit-root
            /tests/lint/story
            /tests/lint/test
            /tests/plan/create
            /tests/plan/show
            /tests/story/create
            /tests/story/show
            /tests/test/create
            /tests/test/show
            /tests/unit/with-development-packages/basic

$ hatch run dev:tmt -c trigger=commit -vv run --max 10 discover plan -n '^/plans/features/core$'
/var/tmp/tmt/run-007
Found 1 plan.

/plans/features/core
summary: Verify core functionality
    discover
        how: fmf
        directory: /home/happz/git/tmt
        hash: 78742f93
        filter: tier: 0, 1 & tag:-provision-only
        summary: 41 tests selected
            /tests/backward-compatibility
            /tests/core/adjust
            /tests/core/context
            /tests/core/debug
            /tests/core/docs
            /tests/core/dry
            /tests/core/enabled
            /tests/core/env
            /tests/core/environment-file
            /tests/core/error
            /tests/core/escaping
            /tests/core/feeling-safe
            /tests/core/fmf-id
            /tests/core/force
            /tests/core/link
            /tests/core/logging
            /tests/core/ls
            /tests/core/order
            /tests/core/path
            /tests/core/phases
            /tests/core/plan-env-file
            /tests/core/smoke
            /tests/core/spaces
            /tests/core/web-link
            /tests/init/base
            /tests/init/full
            /tests/init/git
            /tests/init/mini
            /tests/init/nested
            /tests/lint/all
            /tests/lint/plan/explicit-root
            /tests/lint/plan/implicit-root
            /tests/lint/story
            /tests/lint/test
            /tests/plan/create
            /tests/plan/show
            /tests/story/create
            /tests/story/show
            /tests/test/create
            /tests/test/show
            /tests/unit/with-development-packages/basic

/plans/features/core.1
summary: Verify core functionality
    discover
        status: done
        summary: 10 tests selected
            /tests/backward-compatibility
            /tests/core/adjust
            /tests/core/context
            /tests/core/debug
            /tests/core/docs
            /tests/core/dry
            /tests/core/enabled
            /tests/core/env
            /tests/core/environment-file
            /tests/core/error

/plans/features/core.2
summary: Verify core functionality
    discover
        status: done
        summary: 10 tests selected
            /tests/core/escaping
            /tests/core/feeling-safe
            /tests/core/fmf-id
            /tests/core/force
            /tests/core/link
            /tests/core/logging
            /tests/core/ls
            /tests/core/order
            /tests/core/path
            /tests/core/phases

/plans/features/core.3
summary: Verify core functionality
    discover
        status: done
        summary: 10 tests selected
            /tests/core/plan-env-file
            /tests/core/smoke
            /tests/core/spaces
            /tests/core/web-link
            /tests/init/base
            /tests/init/full
            /tests/init/git
            /tests/init/mini
            /tests/init/nested
            /tests/lint/all

/plans/features/core.4
summary: Verify core functionality
    discover
        status: done
        summary: 10 tests selected
            /tests/lint/plan/explicit-root
            /tests/lint/plan/implicit-root
            /tests/lint/story
            /tests/lint/test
            /tests/plan/create
            /tests/plan/show
            /tests/story/create
            /tests/story/show
            /tests/test/create
            /tests/test/show

/plans/features/core.5
summary: Verify core functionality
    discover
        status: done
        summary: 1 test selected
            /tests/unit/with-development-packages/basic

@happz happz mentioned this pull request May 5, 2024
Open
@LecrisUT
Copy link
Contributor

LecrisUT commented May 6, 2024

Another approach could be to separate only the report step? That way the prepare and finish steps can be shared. execute splitting might not be desired since it would lack prepare/finish, in which case it should use the full splitting like here.

@happz
Copy link
Collaborator Author

happz commented May 6, 2024

Another approach could be to separate only the report step? That way the prepare and finish steps can be shared. execute splitting might not be desired since it would lack prepare/finish, in which case it should use the full splitting like here.

I'm afraid I don't follow, can you elaborate?

@LecrisUT
Copy link
Contributor

LecrisUT commented May 6, 2024

It depends on what the goal for splitting the plans is. If it's for running the tests in parallel then this approach where all test steps are separated prepare, execute, finish, etc.

This can be wasteful if the prepare or finish steps are demanding, e.g. compiling a project. In that case the user would primarily want just the report step to be separate such that testing-farm can group them for navigability.

@happz
Copy link
Collaborator Author

happz commented May 6, 2024

After some tweaks, this prototype works. It's bad, it's crude, and it probably breaks a dozen "must-have" assumptions mentioned in #308, but I'd call it a start. Each subplan gets its own workdir, all subplans have different names, there is no smart aggregation, the original plan still appears in the output, there's no sharing of guests, all plans provision all guests, not just those needed for their tests, the split is based purely on the number of tests discovered, and so on and on and on.

Multiple other methods were proposed for splitting, other use cases, and possibly even combinations of methods (separate by test-level hardware key, sort shared HW requirements into same guests, then apply --max, and so on...)

@happz
Copy link
Collaborator Author

happz commented May 6, 2024

It depends on what the goal for splitting the plans is. If it's for running the tests in parallel then this approach where all test steps are separated prepare, execute, finish, etc.

Plan-level parallelization, i.e. tmt performing more than one plan at the same time, is beyond tmt capabilities right now. It will be needed though - Testing Farm does it and tmt will need to learn to take over this functionality, and some use cases for plan splitting will benefit greatly (and some wouldn't even make much sense without it).

Some of the use cases for the splitting itself mentioned in #308:

  • moving hardware key from plan's provision to test metadata. tmt then should be capable of putting tests with the same (or compatible...) hardware requirements into the same "buckets", then run subplans with subsets of tests.
  • mutually exclusive tests - test A cannot run on the same guest as test B, it would be easier to store this information in test metadata rather than in plan metadata, tmt would then sort them into corresponding buckets, "cloning" the rest of the plan steps.
  • --max aka splitting a large plan into smaller chunks - this one definitely calls for plan-level parallelization: running a large plan "per partes", use more guests to get results quicker.

This can be wasteful if the prepare or finish steps are demanding, e.g. compiling a project. In that case the user would primarily want just the report step to be separate such that testing-farm can group them for navigability.

I assume this scenario is one of those that would benefit from the plan-level parallelization, e.g. one subplan doing its expensive prepare while other subplans are already crunching tests. I'd say this can be already set up with careful use of when, but it'd hit the serial nature of tmt running one plan after another. Definitely related, although I would say we need another issue for this feature, to track it on its own, because even though the yare related, both can be developed separately.

pvalena
pvalena reviewed May 27, 2024
View reviewed changes
tmt/base.py Outdated
@@ -2257,6 +2258,51 @@ def header(self) -> None:
if self.summary:
self.verbose('summary', self.summary, 'green')

# TODO: move these to some place nice, like a plan-splitting plugin. A check
# and a splitter can easily form new family of plugins.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I love this idea.

@happz happz added this to the 1.36 milestone Aug 12, 2024
@happz happz force-pushed the poc-split-plan-by-max branch 2 times, most recently from 1d64b98 to 1080465 Compare August 13, 2024 20:50
@happz happz modified the milestones: 1.36, 1.37 Sep 2, 2024
@happz happz added the command | run tmt run commanda label Sep 2, 2024
@happz happz marked this pull request as ready for review September 2, 2024 08:37
@happz happz added the ci | full test Pull request is ready for the full test execution label Sep 6, 2024
@happz happz marked this pull request as draft September 6, 2024 19:56
@happz happz force-pushed the poc-split-plan-by-max branch 2 times, most recently from 44dea1c to c7ed46d Compare September 20, 2024 12:22
@happz happz marked this pull request as ready for review September 24, 2024 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pvalena pvalena pvalena left review comments

@psss psss Awaiting requested review from psss psss is a code owner

@lukaszachy lukaszachy Awaiting requested review from lukaszachy lukaszachy is a code owner

@thrix thrix Awaiting requested review from thrix thrix is a code owner

@janhavlin janhavlin Awaiting requested review from janhavlin janhavlin is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
ci | full test Pull request is ready for the full test execution command | run tmt run commanda
Projects
None yet
Milestone
1.37
Development

Successfully merging this pull request may close these issues.
3 participants
@happz @LecrisUT @pvalena