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

Multicluster E2E Test Refactoring #3863

Merged
merged 2 commits into from
Jul 7, 2022
Merged

Multicluster E2E Test Refactoring #3863

merged 2 commits into from
Jul 7, 2022

Conversation

GraysonWu
Copy link
Contributor

@GraysonWu GraysonWu commented Jun 7, 2022 •
edited
Loading

Use this PR to replace PR #3286.

Grouped multicluster service export test cases together, and
trigger the test cases from multicluster/test/e2e/main_test.go

Signed-off-by: Enhao Cui [email protected]
Signed-off-by: wgrayson [email protected]

@GraysonWu
Copy link
Contributor Author

/test-multicluster-e2e

@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2022 •
edited
Loading

Codecov Report

Merging #3863 (6bbdb2a) into main (6bbdb2a) will not change coverage.
The diff coverage is n/a.

❗ Current head 6bbdb2a differs from pull request most recent head b3b6b66. Consider uploading reports for the commit b3b6b66 to get more accurate results

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3863   +/-   ##
=======================================
  Coverage   64.44%   64.44%           
=======================================
  Files         294      294           
  Lines       43627    43627           
=======================================
  Hits        28116    28116           
  Misses      13226    13226           
  Partials     2285     2285
Flag Coverage Δ
kind-e2e-tests 50.75% <0.00%> (ø)
unit-tests 44.37% <0.00%> (ø)

@GraysonWu GraysonWu requested review from luolanzone and Dyanngg and removed request for luolanzone June 7, 2022 04:44
@luolanzone
Copy link
Contributor

@GraysonWu thanks for working on e2e refactor, but I am afraid you probably need to wait for this PR #3603 to merge first, there are quite a few e2e changes in this PR, so I would on-hold review until you rebase on latest change.

@GraysonWu
Copy link
Contributor Author

@GraysonWu thanks for working on e2e refactor, but I am afraid you probably need to wait for this PR #3603 to merge first, there are quite a few e2e changes in this PR, so I would on-hold review until you rebase on latest change.

NP

@luolanzone luolanzone added the area/multi-cluster Issues or PRs related to multi cluster. label Jun 8, 2022
@luolanzone luolanzone mentioned this pull request Jun 8, 2022
Closed
@luolanzone
Copy link
Contributor

Hi @GraysonWu Could you help to move this forward? the latest code is merged into main now. thanks.

@GraysonWu
Copy link
Contributor Author

/test-multicluster-e2e

@luolanzone
Copy link
Contributor

/test-multicluster-dataplane-e2e

Hi @GraysonWu , test-multicluster-e2e is deprecated, please use test-multicluster-dataplane-e2e next time, thanks.

luolanzone
luolanzone reviewed Jun 17, 2022
View reviewed changes
multicluster/test/e2e/service_test.go Outdated Show resolved Hide resolved
multicluster/test/e2e/service_test.go Outdated Show resolved Hide resolved
multicluster/test/e2e/service_test.go Outdated Show resolved Hide resolved
multicluster/test/e2e/service_test.go Outdated Show resolved Hide resolved
@luolanzone
Copy link
Contributor

Hi @GraysonWu the e2e job is updated, the new comment is test-multicluster-e2e now. You can use test-multicluster-e2e again to trigger MC e2e.

@GraysonWu
Copy link
Contributor Author

/test-multicluster-e2e

luolanzone
luolanzone reviewed Jul 4, 2022
View reviewed changes
Copy link
Contributor

@luolanzone luolanzone left a comment

Choose a reason for hiding this comment

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

A few nits

multicluster/test/e2e/framework.go Show resolved Hide resolved
multicluster/test/e2e/main_test.go Outdated Show resolved Hide resolved
multicluster/test/e2e/service_test.go Outdated Show resolved Hide resolved
multicluster/test/e2e/service_test.go Outdated Show resolved Hide resolved
multicluster/test/e2e/service_test.go Outdated Show resolved Hide resolved
@luolanzone
Copy link
Contributor

@GraysonWu Could you help to re-base to latest codes and rerun test? thanks.

@GraysonWu
Multicluster E2E Test Refactoring
d4c7b1f 
Grouped multicluster service export test cases together, and
trigger the test cases from multicluster/test/e2e/main_test.go

Signed-off-by: Enhao Cui <[email protected]>
Signed-off-by: wgrayson <[email protected]>
@GraysonWu
Copy link
Contributor Author

@GraysonWu Could you help to re-base to latest codes and rerun test? thanks.

Done. And nits addressed.

@GraysonWu
Copy link
Contributor Author

/test-multicluster-e2e

luolanzone
luolanzone previously approved these changes Jul 6, 2022
View reviewed changes
Copy link
Contributor

@luolanzone luolanzone left a comment

Choose a reason for hiding this comment

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

LGTM

@luolanzone luolanzone requested review from jianjuns and tnqn July 6, 2022 02:50
jianjuns
jianjuns reviewed Jul 6, 2022
View reviewed changes
Copy link
Contributor

@jianjuns jianjuns left a comment

Choose a reason for hiding this comment

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

Nits

multicluster/test/e2e/service_test.go Outdated Show resolved Hide resolved
multicluster/test/e2e/service_test.go Outdated Show resolved Hide resolved
multicluster/test/e2e/service_test.go Outdated Show resolved Hide resolved
@GraysonWu
Address comments
719b313 
Signed-off-by: wgrayson <[email protected]>
@GraysonWu
Copy link
Contributor Author

/test-multicluster-e2e

@jianjuns
Copy link
Contributor

jianjuns commented Jul 7, 2022

/skip-all
The change impacts only multi-cluster e2e tests.

@jianjuns jianjuns merged commit f022dd8 into antrea-io:main Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jianjuns jianjuns jianjuns approved these changes

@luolanzone luolanzone luolanzone left review comments

@Dyanngg Dyanngg Awaiting requested review from Dyanngg

@tnqn tnqn Awaiting requested review from tnqn

Assignees
No one assigned
Labels
area/multi-cluster Issues or PRs related to multi cluster.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@GraysonWu @codecov-commenter @luolanzone @jianjuns