-
Notifications
You must be signed in to change notification settings - Fork 36
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
[JENKINS-72611] Do not update credentials ID in BaseTest
#65
Conversation
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.
this code would pass even if we failed to update the credential. SHould it not change some other part of the credential to ensure that it can be updated (e.g. description?)?
there is also a comment in the code that is no longer correct.
@@ -108,7 +107,7 @@ private <T extends BaseStandardCredentials> void testCreateUpdateDelete(T creden | |||
assertThat(credentials.size(), is(1)); | |||
cred = credentials.get(0); | |||
assertThat(cred, instanceOf(credential.getClass())); | |||
assertThat(cred.getId(), is(UPDATED_CRED_ID)); | |||
assertThat(cred.getId(), is(CRED_ID)); |
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.
check the comment at the top of this block :)
Was gonna push an update to address James' comment. Do I file a new clean up PR or leave as is? |
Oh sorry. A separate PR to clean up the comment would be fine. |
Follow up of jenkinsci/credentials-plugin#506.
PCT currently fails with jenkinsci/credentials-plugin#506 (comment).
Testing done
Submitter checklist