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 Review API Test #456

Merged
merged 5 commits into from
Aug 4, 2023
Merged

🌱 add Review API Test #456

merged 5 commits into from
Aug 4, 2023

Conversation

khareyash05
Copy link
Contributor

Add review API Test

Work to be Done

  1. Check for Review.List()
  2. Check for Copy Root

Signed-off-by: Yash Khare <yash2010118@akgec.ac.in>
Signed-off-by: Yash Khare <yash2010118@akgec.ac.in>
Copy link
Member

@aufi aufi left a comment

Choose a reason for hiding this comment

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

Overall looks good! Left few comments.

binding/review.go Outdated Show resolved Hide resolved
test/api/review/api_test.go Outdated Show resolved Hide resolved
test/api/review/api_test.go Outdated Show resolved Hide resolved
@aufi
Copy link
Member

aufi commented Jul 28, 2023

Please don't forget update https://github.com/konveyor/tackle2-hub/blob/main/docs/test-api-matrix.md

@aufi
Copy link
Member

aufi commented Jul 28, 2023

Related to konveyor/operator#220

Signed-off-by: Yash Khare <yash2010118@akgec.ac.in>
@khareyash05 khareyash05 requested a review from aufi July 29, 2023 06:11
Signed-off-by: Yash Khare <yash2010118@akgec.ac.in>
@khareyash05 khareyash05 marked this pull request as ready for review August 1, 2023 04:58
Copy link
Member

@aufi aufi left a comment

Choose a reason for hiding this comment

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

Commented few things I missed before, otherwise looks good to me!

assert.Must(t, Application.Delete(app.ID))
})

t.Run("Delete Review and its dependencies", func(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why is application delete before the reviews (so why there is a separated subtets for the deletion).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will merge the test!

}
assert.Must(t, Application.Create(&destApp))

err := Review.Copy(r.ID, destApp.ID)
Copy link
Member

Choose a reason for hiding this comment

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

It cold be better check the copied review fields with AssertEqualReviews

Signed-off-by: Yash Khare <yash2010118@akgec.ac.in>
Copy link
Member

@aufi aufi left a comment

Choose a reason for hiding this comment

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

Looks good to me! Will merge when #467 gets in.

@aufi aufi closed this Aug 4, 2023
@aufi aufi reopened this Aug 4, 2023
@aufi aufi merged commit b05ee5e into konveyor:main Aug 4, 2023
14 of 15 checks passed
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