Update tests wrt ginkgo 2: Add Serial decorators and change checks for serial run #8770

dhiller · 2022-11-11T16:26:28Z

What this PR does / why we need it:

Ginkgo V2 has the decorator Serial that is designed for running tests in serial. Further the docs for parallel test support say that test results will all be captured in one output stream in Ginkgo 2.

As a preparation to use the new features this PR:

adds the Serial decorator,
updates the checks for serial run and
updates documentation so that people don't forget when they add or update tests.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

See #8776

Release note:

Add Ginkgo V2 Serial decorator to serial tests as preparation to simplify parallel vs. serial test run logic

kubevirt-bot · 2022-11-11T16:26:30Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

dhiller · 2022-11-11T16:42:00Z

/test pull-kubevirt-e2e-k8s-1.24-sig-compute
/test pull-kubevirt-e2e-k8s-1.24-sig-compute-migrations
/test pull-kubevirt-e2e-k8s-1.24-sig-network
/test pull-kubevirt-e2e-k8s-1.24-sig-storage
/test pull-kubevirt-e2e-kind-1.22-sriov

oshoval

Thanks

Few notes:
It makes KUBEVIRT_E2E_RUN_ALL_SUITES obsolete and always true
as it will run the serial even that the parallel is failed (didnt validate but i believe this is the behavior)
(the periodic lanes are the ones that use it from what i remember)

Once junit merger is removed, additional changes should be done for the unit tests
as they run parallel and use junit merger (saw that junit merger removal is planned for other PR).
(once it is dropped, we can drop staging/src/kubevirt.io/client-go/reporter/reporter.go and use the new built in junit-reporter instead junit-output IIRC)

Should we also remove the [Serial] manual label ?
It is not needed, but might left for helping readability ? (note that it is redundant and might be nicer to remove it)
Unless it allows to run manually only the serial ones, with focus style?
(or vice versa to skip just the serial ones when running locally)
Better that we will have options for those

oshoval · 2022-11-14T07:13:09Z

hack/functests.sh

 fi
+
+functest ${test_args} ${KUBEVIRT_FUNC_TEST_GINKGO_ARGS}


don't we need to use --junit-report ( --output-dir might be needed as well) ?
and then to remove the call to the custom reporter at tests/tests_suite_test.go

@oshoval thanks for your review and the reminders.

Regarding the flag: IIRC no, we don't need to use the flag here, since it has a default value. But we can't drop it for backwards compatibility of other builds (like i.e. our d/s build), who are using this one to override the test output file name.

Regarding the custom reporter, thanks for the hint, will look into it.

dhiller · 2022-11-14T11:08:27Z

/test pull-kubevirt-e2e-k8s-1.24-sig-compute
/test pull-kubevirt-e2e-k8s-1.24-sig-compute-migrations

dhiller · 2022-11-15T10:10:05Z

/test pull-kubevirt-e2e-k8s-1.24-sig-compute
/test pull-kubevirt-e2e-k8s-1.24-sig-compute-migrations
/test pull-kubevirt-e2e-k8s-1.24-sig-network
/test pull-kubevirt-e2e-k8s-1.24-sig-storage
/test pull-kubevirt-e2e-kind-1.22-sriov

See https://onsi.github.io/ginkgo/MIGRATING_TO_V2#serial-decorator Signed-off-by: Daniel Hiller <[email protected]>

Tests that check for running in serial need to get adapted to use info from the current spec, since there's no more separate running, thus the ParallelTotal will stay the same, even if the test is run in Serial mode. Signed-off-by: Daniel Hiller <[email protected]>

Signed-off-by: Daniel Hiller <[email protected]>

dhiller · 2022-11-15T12:31:49Z

/test pull-kubevirt-e2e-k8s-1.24-sig-compute
/test pull-kubevirt-e2e-k8s-1.24-sig-compute-migrations

enp0s3

@dhiller Thank you for the clarity !
/approve

kubevirt-bot · 2022-11-17T12:35:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enp0s3

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~tests/OWNERS~~ [enp0s3]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

oshoval · 2022-11-17T12:42:15Z

I am a bit lost about his PR and #8788
can you please elaborate which commits there are unique and what is common ?

You consider this PR as a working standalone ?
are we safe despite we still also call the old reporter as well ?
maybe better to remove it in this PR ? just the calls to the old reporter from the e2e tests

enp0s3 · 2022-11-17T13:03:25Z

@oshoval From what I understand, this PR only prepares the ground for using Ginkgo parallel testing by just decorating with Serial the tests that should run in serial. Current PR won't replace our mechanism of Parallel and Serial testing.

oshoval · 2022-11-17T13:04:47Z

@oshoval From what I understand, this PR only prepares the ground for using Ginkgo parallel testing by just decorating with Serial the tests that should run in serial. Current PR won't replace our mechanism of Parallel and Serial testing.

ah right, commits that were here where moved

enp0s3 · 2022-11-17T13:04:49Z

@oshoval The separation also announced in #8776

oshoval

Thanks

PR desc
"updates the checks for serial run and"
the word and is not needed

This is a backport of the changes introduced here: kubevirt#8770. Signed-off-by: Itamar Holder <[email protected]>

kubevirt-bot requested review from kbidarkar, phoracek and rthallisey November 11, 2022 16:26

kubevirt-bot added size/L sig/network labels Nov 11, 2022

dhiller changed the title ~~Update parallel test logic wrt ginkgo 2~~ Simplify running parallel and serial tests wrt ginkgo 2 Nov 11, 2022

oshoval reviewed Nov 13, 2022

View reviewed changes

kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 13, 2022

oshoval reviewed Nov 14, 2022

View reviewed changes

dhiller force-pushed the update-parallel-test-logic-wrt-ginkgo-2 branch 2 times, most recently from 9e5bc01 to eb2331e Compare November 14, 2022 11:07

kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 14, 2022

dhiller mentioned this pull request Nov 14, 2022

Simplify running parallel and serial tests wrt ginkgo 2 #8776

Closed

6 tasks

dhiller force-pushed the update-parallel-test-logic-wrt-ginkgo-2 branch from eb2331e to 74becdc Compare November 15, 2022 09:59

dhiller mentioned this pull request Nov 15, 2022

Update tests wrt ginkgo 2: remove scripting running parallel vs. serial tests #8788

Closed

dhiller added 3 commits November 15, 2022 13:29

Add Serial decorator to mark tests for Ginkgo v2

242cc3b

See https://onsi.github.io/ginkgo/MIGRATING_TO_V2#serial-decorator Signed-off-by: Daniel Hiller <[email protected]>

Update documentation on how to write serial tests

9eb3d11

Signed-off-by: Daniel Hiller <[email protected]>

dhiller force-pushed the update-parallel-test-logic-wrt-ginkgo-2 branch from 74becdc to 9eb3d11 Compare November 15, 2022 12:30

dhiller changed the title ~~Simplify running parallel and serial tests wrt ginkgo 2~~ Add Serial decorators and change checks for serial run - tests wrt ginkgo 2 Nov 15, 2022

dhiller changed the title ~~Add Serial decorators and change checks for serial run - tests wrt ginkgo 2~~ Update tests wrt ginkgo 2: Add Serial decorators and change checks for serial run Nov 15, 2022

dhiller marked this pull request as ready for review November 16, 2022 09:12

kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 16, 2022

kubevirt-bot requested a review from EdDev November 16, 2022 09:12

dhiller requested review from xpivarc and enp0s3 and removed request for phoracek, rthallisey and EdDev November 16, 2022 10:21

enp0s3 reviewed Nov 17, 2022

View reviewed changes

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 17, 2022

oshoval approved these changes Nov 17, 2022

View reviewed changes

kubevirt-bot assigned oshoval Nov 17, 2022

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 17, 2022

kubevirt-bot merged commit 55d7144 into kubevirt:main Nov 18, 2022

iholder101 added a commit to iholder101/kubevirt that referenced this pull request Jan 3, 2023

Use CurrentSpecReport().IsSerial to check for Serial run

67cc635

This is a backport of the changes introduced here: kubevirt#8770. Signed-off-by: Itamar Holder <[email protected]>

iholder101 mentioned this pull request Jan 3, 2023

[release-0.53]: Use CurrentSpecReport().IsSerial to check for Serial run #9009

Closed

dhiller mentioned this pull request Oct 12, 2023

[rebased] Update tests wrt ginkgo 2: remove scripting running parallel vs. serial tests #10562

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update tests wrt ginkgo 2: Add Serial decorators and change checks for serial run #8770

Update tests wrt ginkgo 2: Add Serial decorators and change checks for serial run #8770

dhiller commented Nov 11, 2022 •

edited

Loading

kubevirt-bot commented Nov 11, 2022

dhiller commented Nov 11, 2022

oshoval left a comment •

edited

Loading

oshoval Nov 14, 2022 •

edited

Loading

dhiller Nov 14, 2022

dhiller commented Nov 14, 2022

dhiller commented Nov 15, 2022

dhiller commented Nov 15, 2022

enp0s3 left a comment •

edited

Loading

kubevirt-bot commented Nov 17, 2022

oshoval commented Nov 17, 2022 •

edited

Loading

enp0s3 commented Nov 17, 2022

oshoval commented Nov 17, 2022

enp0s3 commented Nov 17, 2022

oshoval left a comment

Update tests wrt ginkgo 2: Add Serial decorators and change checks for serial run #8770

Update tests wrt ginkgo 2: Add Serial decorators and change checks for serial run #8770

Conversation

dhiller commented Nov 11, 2022 • edited Loading

kubevirt-bot commented Nov 11, 2022

dhiller commented Nov 11, 2022

oshoval left a comment • edited Loading

Choose a reason for hiding this comment

oshoval Nov 14, 2022 • edited Loading

Choose a reason for hiding this comment

dhiller Nov 14, 2022

Choose a reason for hiding this comment

dhiller commented Nov 14, 2022

dhiller commented Nov 15, 2022

dhiller commented Nov 15, 2022

enp0s3 left a comment • edited Loading

Choose a reason for hiding this comment

kubevirt-bot commented Nov 17, 2022

oshoval commented Nov 17, 2022 • edited Loading

enp0s3 commented Nov 17, 2022

oshoval commented Nov 17, 2022

enp0s3 commented Nov 17, 2022

oshoval left a comment

Choose a reason for hiding this comment

dhiller commented Nov 11, 2022 •

edited

Loading

oshoval left a comment •

edited

Loading

oshoval Nov 14, 2022 •

edited

Loading

enp0s3 left a comment •

edited

Loading

oshoval commented Nov 17, 2022 •

edited

Loading