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

chore(RHTAPATCH-1069): unit-tests for runAccessCheck #36

Merged
merged 1 commit into from
Jul 21, 2024

Conversation

Omeramsc
Copy link
Collaborator

@Omeramsc Omeramsc commented Jul 2, 2024

Add unit-tests to runAccessCheck.
We decided to perform the tests using the same test suite
we are using for the functional tests, using testenv.
It is setting up a real environment for us, and we don't
have to deal with ugly fakes when using the k8s authorization.

@Omeramsc Omeramsc force-pushed the RHTAPWATCH-1069 branch 2 times, most recently from 3264d4a to 5f99716 Compare July 15, 2024 15:09
@Omeramsc Omeramsc marked this pull request as ready for review July 15, 2024 15:11
Copy link
Member

@ifireball ifireball left a comment

Choose a reason for hiding this comment

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

lgtm

cmd/main_test.go Outdated Show resolved Hide resolved
@yftacherzog
Copy link
Member

/retest

1 similar comment
@yftacherzog
Copy link
Member

/retest

Copy link
Contributor

@hmariset hmariset left a comment

Choose a reason for hiding this comment

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

lgtm

@Omeramsc Omeramsc requested a review from yftacherzog July 21, 2024 06:49
Copy link
Member

@yftacherzog yftacherzog left a comment

Choose a reason for hiding this comment

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

lgtm. fix the typos on the descriptions.

cmd/main_test.go Outdated
Expect(err).NotTo(HaveOccurred(), "Unexpected error testing RunAccessCheck")
},
Entry(
"A user that has access to the resource should return true (user2 have permission to 'create' on test-tenant-1)",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"A user that has access to the resource should return true (user2 have permission to 'create' on test-tenant-1)",
"A user that has access to the resource should return true (user2 has permission to 'create' on test-tenant-1)",

cmd/main_test.go Outdated
"create",
true),
Entry(
"A user that does not have any premissions on the namespace should return false (user1 don't have access to test-tenant-2)",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"A user that does not have any premissions on the namespace should return false (user1 don't have access to test-tenant-2)",
"A user that does not have any premissions on the namespace should return false (user1 doesn't have access to test-tenant-2)",

cmd/main_test.go Outdated
"create",
false),
Entry(
"A user that does not have the permissions to perform the specific action on the namespace should return false (user1 don't have permission to 'patch' on test-tenant-1)",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"A user that does not have the permissions to perform the specific action on the namespace should return false (user1 don't have permission to 'patch' on test-tenant-1)",
"A user that does not have the permissions to perform the specific action on the namespace should return false (user1 doesn't have permission to 'patch' on test-tenant-1)",

Add unit-tests to `runAccessCheck`.
We decided to perform the tests using the same test suite
we are using for the functional tests, using testenv.
It is setting up a real environment for us, and we don't
have to deal with ugly fakes when using the k8s authorization.

Signed-off-by: Omer Turner <[email protected]>
@yftacherzog yftacherzog merged commit c226256 into konflux-ci:main Jul 21, 2024
7 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.

4 participants