-
Notifications
You must be signed in to change notification settings - Fork 743
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: Refactor tests to utilize github.com/stretchr/testify #509
Conversation
Signed-off-by: Tyler Auerbeck <tylerauerbeck@users.noreply.github.com>
Signed-off-by: Tyler Auerbeck <tylerauerbeck@users.noreply.github.com>
Signed-off-by: Tyler Auerbeck <tylerauerbeck@users.noreply.github.com>
Signed-off-by: Tyler Auerbeck <tylerauerbeck@users.noreply.github.com>
Signed-off-by: Tyler Auerbeck <tylerauerbeck@users.noreply.github.com>
Signed-off-by: Tyler Auerbeck <tylerauerbeck@users.noreply.github.com>
Signed-off-by: Tyler Auerbeck <tylerauerbeck@users.noreply.github.com>
|
||
// Check if the CLuster Role gets deleted | ||
os.Unsetenv("ARGOCD_CLUSTER_CONFIG_NAMESPACES") | ||
_, err = r.reconcileClusterRole(workloadIdentifier, expectedRules, a) | ||
assert.ErrorContains(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: clusterRoleName}, reconciledClusterRole), "not found") | ||
//assert.ErrorContains(t, r.Client.Get(context.TODO(), types.NamespacedName{Name: clusterRoleName}, reconciledClusterRole), "not found") | ||
//TODO: https://github.com/stretchr/testify/pull/1022 introduced ErrorContains, but is not yet available in a tagged release. Revert to ErrorContains once this becomes available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment still valid? It looks like that PR has been merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. Still valid. It's been merged. But doesn't look like it's made its way into a versioned release of testify yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
thanks @tylerauerbeck
What type of PR is this?
/kind code-refactoring
What does this PR do / why we need it:
This PR continues the initial work to refactor tests to utilize github.com/stretchr/testify/assert
Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Continues to address #302
How to test changes / Special notes to the reviewer:
make test
**Additional Info:
There are tests that can be further refactored at this point, but the primary point of this PR is to at least standardize on the github.com/testify/assert library throughout all of the tests prior to making any larger changes.