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.

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

add tests for prepareForDataSynchronization func #52

Merged
merged 4 commits into from
Oct 28, 2024

Conversation

peng225
Copy link
Contributor

@peng225 peng225 commented Oct 18, 2024

Signed-off-by: Shinya Hayashi shinya-hayashi@cybozu.co.jp

@peng225 peng225 force-pushed the additional-unit-test-for-prepare-data-sync branch 8 times, most recently from 81de935 to 422e9f9 Compare October 22, 2024 07:50
@peng225 peng225 marked this pull request as ready for review October 22, 2024 07:54
Expect(ctrlClient).NotTo(BeNil())
primaryBackups := append(primaryBackupsWithoutTarget, backup.DeepCopy())
for _, backup := range primaryBackups {
shouldbeDeleted := false
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
shouldbeDeleted := false
shouldBeDeleted := false

Copy link
Contributor Author

Choose a reason for hiding this comment

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


var _ = Describe("prepareForDataSynchronization", func() {
testPVCUID := "d3b07384-d9a7-4e6b-8a3b-1f4b7b7b7b7b"
testName := "test5"
Copy link
Contributor

Choose a reason for hiding this comment

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

I belive "testSnap5" is better since "5" doesn't mean just a unique number among all tests but is equal to the snapID. Other testNames, "testN" should also be changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@peng225 peng225 force-pushed the additional-unit-test-for-prepare-data-sync branch from 422e9f9 to a0e5141 Compare October 28, 2024 01:30
Signed-off-by: Shinya Hayashi <shinya-hayashi@cybozu.co.jp>
Signed-off-by: Shinya Hayashi <shinya-hayashi@cybozu.co.jp>
@peng225 peng225 force-pushed the additional-unit-test-for-prepare-data-sync branch from a0e5141 to b1f8ee9 Compare October 28, 2024 01:34
Signed-off-by: Shinya Hayashi <shinya-hayashi@cybozu.co.jp>
Signed-off-by: Shinya Hayashi <shinya-hayashi@cybozu.co.jp>
@satoru-takeuchi
Copy link
Contributor

Although build workflow failed, it seems not to be relatated to this PR. The test fails on creating minikube cluster. I re-ran this workflow.

Copy link
Contributor

@satoru-takeuchi satoru-takeuchi left a comment

Choose a reason for hiding this comment

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

@peng225 LGTM. Do you plan to squash commits? If else, I'll merge this PR after CI succeeded.

@peng225
Copy link
Contributor Author

peng225 commented Oct 28, 2024

Do you plan to squash commits?

No, I don't. Now that CI has passed, I'll merge it.

@peng225 peng225 merged commit f78824d into main Oct 28, 2024
3 checks passed
@peng225 peng225 deleted the additional-unit-test-for-prepare-data-sync branch October 28, 2024 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants