simplified logic and add test case #3422

0xff-dev · 2022-07-05T06:07:35Z

Signed-off-by: 0xff-dev stevenshuang521@gmail.com

What problem does this PR solve?

simplified logic and add test case.

What's changed and how it works?

return in advance when conditions are met to avoid redundant traversal.

Related changes

Need to update chaos-mesh/website
Need to update Dashboard UI
Need to cheery-pick to release branches
- release-2.2
- release-2.1

Checklist

CHANGELOG

I have updated the CHANGELOG.md
I have labeled this PR with "no-need-update-changelog"

Tests

Unit test
E2E test
No code
Manual test (add steps below)

Side effects

Breaking backward compatibility

DCO

If you find the DCO check fails, please run commands like below (Depends on the actual situations. For example, if the failed commit isn't the most recent) to fix it:

git commit --amend --signoff
git push --force

ti-chi-bot · 2022-07-05T06:07:37Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

STRRL
cwen0

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

ti-chi-bot · 2022-07-05T06:07:44Z

Welcome @0xff-dev!

It looks like this is your first PR to chaos-mesh/chaos-mesh 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to chaos-mesh/chaos-mesh. 😃

codecov · 2022-07-06T07:03:23Z

Codecov Report

Merging #3422 (813007c) into master (ea12c18) will decrease coverage by 0.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3422      +/-   ##
==========================================
- Coverage   40.82%   40.70%   -0.13%     
==========================================
  Files         166      166              
  Lines       13990    13985       -5     
==========================================
- Hits         5711     5692      -19     
- Misses       7849     7860      +11     
- Partials      430      433       +3

Impacted Files	Coverage Δ
pkg/finalizer/utils.go	`100.00% <100.00%> (+5.55%)`	⬆️
.../workflow/controllers/abort_workflow_reconciler.go	`48.97% <0.00%> (-8.17%)`	⬇️
pkg/workflow/controllers/chaos_node_reconciler.go	`58.16% <0.00%> (-3.16%)`	⬇️
.../workflow/controllers/workflow_entry_reconciler.go	`47.02% <0.00%> (-2.17%)`	⬇️
controllers/statuscheck/controller.go	`84.14% <0.00%> (+4.87%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40d7f06...813007c. Read the comment docs.

cwen0

LGTM

STRRL

LGTM!

STRRL · 2022-07-07T06:09:01Z

Thanks for your contribution! @0xff-dev

STRRL · 2022-07-07T06:12:12Z

Hi @0xff-dev, please resolve the conflicts with CHANGELOG.md. And I prefer to change the content in changelog like

Simplified logic and add test case about finalizers. [#3422](https://github.com/chaos-mesh/chaos-mesh/pull/3422)

0xff-dev · 2022-07-07T06:13:34Z

Hi @0xff-dev, please resolve the conflicts with CHANGELOG.md. And I prefer to change the content in changelog like
Simplified logic and add test case about finalizers. [#3422](https://github.com/chaos-mesh/chaos-mesh/pull/3422)

ok.

STRRL · 2022-07-07T06:43:03Z

Hi @0xff-dev, please resolve the conflicts with CHANGELOG.md. And I prefer to change the content in changelog like
Simplified logic and add test case about finalizers. [#3422](https://github.com/chaos-mesh/chaos-mesh/pull/3422[](https://github.com/chaos-mesh/chaos-mesh/pull/3422))
ok.

Hi @0xff-dev ! How about updating the changelog? 🤣

Signed-off-by: 0xff-dev <stevenshuang521@gmail.com>

0xff-dev · 2022-07-07T06:47:54Z

Hi @0xff-dev, please resolve the conflicts with CHANGELOG.md. And I prefer to change the content in changelog like
Simplified logic and add test case about finalizers. [#3422](https://github.com/chaos-mesh/chaos-mesh/pull/3422[](https://github.com/chaos-mesh/chaos-mesh/pull/3422))
ok.
Hi @0xff-dev ! How about updating the changelog? 🤣
I'm so sorry, I just resolved the conflict. changelog has been updated

0xff-dev · 2022-07-07T06:49:13Z

Hi @0xff-dev, please resolve the conflicts with CHANGELOG.md. And I prefer to change the content in changelog like
Simplified logic and add test case about finalizers. [#3422](https://github.com/chaos-mesh/chaos-mesh/pull/3422[](https://github.com/chaos-mesh/chaos-mesh/pull/3422))
ok.
Hi @0xff-dev ! How about updating the changelog? 🤣

First contribution, a little excited. 😅

STRRL · 2022-07-07T06:58:46Z

Hi @0xff-dev, please resolve the conflicts with CHANGELOG.md. And I prefer to change the content in changelog like
Simplified logic and add test case about finalizers. [#3422](https://github.com/chaos-mesh/chaos-mesh/pull/3422[](https://github.com/chaos-mesh/chaos-mesh/pull/3422)[](https://github.com/chaos-mesh/chaos-mesh/pull/3422[](https://github.com/chaos-mesh/chaos-mesh/pull/3422)))
ok.
Hi @0xff-dev ! How about updating the changelog? 🤣
First contribution, a little excited. 😅

Haha, again, thanks for your contribution!

STRRL · 2022-07-07T06:58:59Z

/merge

ti-chi-bot · 2022-07-07T06:59:02Z

This pull request has been accepted and is ready to merge.

Commit hash: 404394f

ti-chi-bot · 2022-07-07T08:33:17Z

@0xff-dev: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

Signed-off-by: 0xff-dev <stevenshuang521@gmail.com> Co-authored-by: Ti Chi Robot <ti-community-prow-bot@tidb.io> Signed-off-by: STRRL <im@strrl.dev>

ti-chi-bot added contribution first-time-contributor labels Jul 5, 2022

ti-chi-bot requested a review from yeya24 July 5, 2022 06:07

ti-chi-bot added the size/S label Jul 5, 2022

0xff-dev force-pushed the master branch from e8e9cb7 to 7679698 Compare July 5, 2022 06:08

cwen0 approved these changes Jul 6, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 label Jul 6, 2022

STRRL approved these changes Jul 7, 2022

View reviewed changes

ti-chi-bot added status/LGT2 needs-rebase and removed status/LGT1 labels Jul 7, 2022

0xff-dev force-pushed the master branch from 7679698 to 33b2285 Compare July 7, 2022 06:15

ti-chi-bot removed the needs-rebase label Jul 7, 2022

0xff-dev requested a review from STRRL July 7, 2022 06:16

0xff-dev force-pushed the master branch from 33b2285 to 51a7b99 Compare July 7, 2022 06:20

simplified logic and add test case

404394f

Signed-off-by: 0xff-dev <stevenshuang521@gmail.com>

0xff-dev force-pushed the master branch from 51a7b99 to 404394f Compare July 7, 2022 06:47

ti-chi-bot added the status/can-merge label Jul 7, 2022

ti-chi-bot added 2 commits July 7, 2022 16:17

Merge branch 'master' into master

6ac9472

Merge branch 'master' into master

813007c

ti-chi-bot merged commit 9522623 into chaos-mesh:master Jul 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

simplified logic and add test case #3422

simplified logic and add test case #3422

0xff-dev commented Jul 5, 2022

ti-chi-bot commented Jul 5, 2022 •

edited

ti-chi-bot commented Jul 5, 2022

codecov bot commented Jul 6, 2022 •

edited

cwen0 left a comment

STRRL left a comment

STRRL commented Jul 7, 2022 •

edited

STRRL commented Jul 7, 2022

0xff-dev commented Jul 7, 2022

STRRL commented Jul 7, 2022

0xff-dev commented Jul 7, 2022

0xff-dev commented Jul 7, 2022

STRRL commented Jul 7, 2022

STRRL commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

simplified logic and add test case #3422

simplified logic and add test case #3422

Conversation

0xff-dev commented Jul 5, 2022

What problem does this PR solve?

What's changed and how it works?

Related changes

Checklist

DCO

ti-chi-bot commented Jul 5, 2022 • edited

ti-chi-bot commented Jul 5, 2022

codecov bot commented Jul 6, 2022 • edited

Codecov Report

cwen0 left a comment

Choose a reason for hiding this comment

STRRL left a comment

Choose a reason for hiding this comment

STRRL commented Jul 7, 2022 • edited

STRRL commented Jul 7, 2022

0xff-dev commented Jul 7, 2022

STRRL commented Jul 7, 2022

0xff-dev commented Jul 7, 2022

0xff-dev commented Jul 7, 2022

STRRL commented Jul 7, 2022

STRRL commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

ti-chi-bot commented Jul 7, 2022

ti-chi-bot commented Jul 5, 2022 •

edited

codecov bot commented Jul 6, 2022 •

edited

STRRL commented Jul 7, 2022 •

edited