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

Introduce multikueue integration test target for separate CI run #4042

Merged
merged 4 commits into from
Jan 24, 2025
Merged

Introduce multikueue integration test target for separate CI run #4042

merged 4 commits into from
Jan 24, 2025

Conversation

mszadkow
Copy link
Contributor

What type of PR is this?

/kind feature

What this PR does / why we need it:

The multikueue integration tests are quite heavy in nature.
Thus to meet the demand for new integrations in multikueue in the future we need to separate them from the existing tests.

Which issue(s) this PR fixes:

Relates to #4040

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

@k8s-ci-robot
Copy link
Contributor

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

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 23, 2025
@mszadkow
Copy link
Contributor Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 23, 2025
@netlify Netlify
Copy link

netlify bot commented Jan 23, 2025 •
edited
Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
🔨 Latest commit 67895b7
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/679396944fa7c80008c94a89

@mszadkow mszadkow force-pushed the feature/extract-multikueue-integration-test branch 2 times, most recently from 879fefe to 64b5584 Compare January 23, 2025 16:43
mszadkow
mszadkow commented Jan 23, 2025
View reviewed changes
Makefile-test.mk Outdated Show resolved Hide resolved
@mszadkow mszadkow marked this pull request as ready for review January 23, 2025 16:44
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 23, 2025
@k8s-ci-robot k8s-ci-robot requested a review from tenzen-y January 23, 2025 16:44
@mimowo
Copy link
Contributor

mimowo commented Jan 23, 2025 •
edited
Loading

@mszadkow would it be easier to nest the structure of the suites to have singlecluster and multicluster grouping folders as for e2e tests? How otherwise are we to run the main integration tests, excluding multkueue?

@mszadkow
Copy link
Contributor Author

@mszadkow would it be easier to nest the structure of the suites to have singlecluster and multicluster grouping folders as for e2e tests? How otherwise are we to run the main integration tests, excluding multkueue?

Yes, all is possible.
You guys voting for separate folders? np

mbobrovskyi
mbobrovskyi reviewed Jan 23, 2025
View reviewed changes
Makefile-test.mk Outdated Show resolved Hide resolved
@mszadkow mszadkow force-pushed the feature/extract-multikueue-integration-test branch from 64b5584 to 886897d Compare January 23, 2025 18:05
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 23, 2025
@mszadkow
Copy link
Contributor Author

Answering your question about file path.Join and usage of PROJECT_DIR.
Either this or I have to add there another level of indirection.
This change is beneficial also for readability stand point imho

@mszadkow mszadkow force-pushed the feature/extract-multikueue-integration-test branch from 17759f3 to e44d8ec Compare January 24, 2025 08:00
mimowo
mimowo reviewed Jan 24, 2025
View reviewed changes
Copy link
Contributor

@mimowo mimowo left a comment

Choose a reason for hiding this comment

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

Lgtm overall

test/integration/multikueue/suite_test.go Outdated Show resolved Hide resolved
@mszadkow mszadkow force-pushed the feature/extract-multikueue-integration-test branch from e44d8ec to 30a43cd Compare January 24, 2025 12:19
@mimowo
Copy link
Contributor

mimowo commented Jan 24, 2025

Yes, all is possible.
You guys voting for separate folders? np

I see, I don't have a strong view on that, but it feels more consistent with what we do for e2e tests.

@mimowo
Copy link
Contributor

mimowo commented Jan 24, 2025

/assign @mbobrovskyi
please review

@mimowo mimowo mentioned this pull request Jan 24, 2025
Open
@mszadkow mszadkow force-pushed the feature/extract-multikueue-integration-test branch from 30a43cd to 67895b7 Compare January 24, 2025 13:33
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 24, 2025
mbobrovskyi
mbobrovskyi approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@mbobrovskyi mbobrovskyi left a comment

Choose a reason for hiding this comment

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

/lgtm
Thanks!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 24, 2025
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: c841c5f35159868c715506f99345c6f82877fb70

@mimowo
Copy link
Contributor

mimowo commented Jan 24, 2025

/approve
Great, thanks!

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mbobrovskyi, mimowo, mszadkow

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 24, 2025
@k8s-ci-robot k8s-ci-robot merged commit fffb1ab into kubernetes-sigs:main Jan 24, 2025
17 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v0.11 milestone Jan 24, 2025
@mszadkow mszadkow deleted the feature/extract-multikueue-integration-test branch January 24, 2025 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mimowo mimowo mimowo left review comments

@mbobrovskyi mbobrovskyi mbobrovskyi approved these changes

@PBundyra PBundyra Awaiting requested review from PBundyra

@tenzen-y tenzen-y Awaiting requested review from tenzen-y

Assignees

@mbobrovskyi mbobrovskyi

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v0.11
Development

Successfully merging this pull request may close these issues.
4 participants
@mszadkow @k8s-ci-robot @mimowo @mbobrovskyi