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

[DRAFT] Add framework for global role integration tests #45450

Draft
wants to merge 3 commits into
base: release/v2.9
Choose a base branch
from

Conversation

JonCrowther
Copy link
Contributor

Issue:

Problem

Solution

Testing

Engineering Testing

Manual Testing

Automated Testing

  • Test types added/modified:
    • Unit
    • Integration (Go Framework)
    • Integration (v2prov Framework)
    • Validation (Go Framework)
    • Other - Explain: EXPLAIN
    • None
    • REMOVE NOT APPLICABLE BULLET POINTS ABOVE
  • If "None" - Reason: EXPLAIN THE REASON
  • If "None" - GH Issue/PR: LINK TO GH ISSUE/PR TO ADD TESTS

Summary: TODO

QA Testing Considerations

Regressions Considerations

TODO

Existing / newly added automated tests that provide evidence there are no regressions:

  • TODO

@JonCrowther JonCrowther force-pushed the global-roles-integration-tests branch from d0c4442 to 752d0f5 Compare June 5, 2024 16:52
Copy link
Contributor

@raulcabello raulcabello left a comment

Choose a reason for hiding this comment

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

It looks great! I left some small comments.
Thanks Jonathan!

name: "create primary cluster role with generated name",
globalRole: v3.GlobalRole{
ObjectMeta: metav1.ObjectMeta{
Name: "test-gr",
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use a different name to avoid conflics with the previous test. I got an error saying that the GlobalRole existed because it was still deleting the previous one.

assert.NoError(t, err)

// Assert any desired role fields
assert.Equal(t, test.globalRole.Name, role.OwnerReferences[0].Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think we can remove this as this is tested in line 218

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.

None yet

2 participants