Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Forward compatibility with jenkinsci/credentials-plugin#551 #1756

Merged
merged 4 commits into from
Oct 6, 2024

Conversation

janfaracik
Copy link
Contributor

jenkinsci/credentials-plugin#551

Testing done

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@timja
Copy link
Member

timja commented Oct 6, 2024

(can ignore JiraPluginTest failing)

@basil
Copy link
Member

basil commented Oct 6, 2024

when updating ATH tests for plugin changes, do we need to maintain compatibility with the old version of the plugin?

@janfaracik I think it is desirable to preserve compatibility unless it is prohibitively difficult. This PR passed against jenkinsci/credentials-plugin#551 but is failing on the current release of the Credentials plugin with

java.lang.IndexOutOfBoundsException: Index 1 out of bounds for length 0
	at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64)
	at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70)
	at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:266)
	at java.base/java.util.Objects.checkIndex(Objects.java:361)
	at java.base/java.util.ArrayList.get(ArrayList.java:427)
	at org.jenkinsci.test.acceptance.plugins.ssh_slaves.SshSlaveLauncher.addCredential(SshSlaveLauncher.java:39)
	at org.jenkinsci.test.acceptance.plugins.ssh_slaves.SshSlaveLauncher.pwdCredentials(SshSlaveLauncher.java:69)

In this case I think it would be relatively easy to support both cases with some sort of if/else or try/catch statement, so I think we should strive to preserve compatibility. See #1690 and #1693 for plenty of examples of how this was done before.

@janfaracik janfaracik marked this pull request as ready for review October 6, 2024 18:47
@janfaracik
Copy link
Contributor Author

when updating ATH tests for plugin changes, do we need to maintain compatibility with the old version of the plugin?

@janfaracik I think it is desirable to preserve compatibility unless it is prohibitively difficult. This PR passed against jenkinsci/credentials-plugin#551 but is failing on the current release of the Credentials plugin with

java.lang.IndexOutOfBoundsException: Index 1 out of bounds for length 0
	at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64)
	at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70)
	at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:266)
	at java.base/java.util.Objects.checkIndex(Objects.java:361)
	at java.base/java.util.ArrayList.get(ArrayList.java:427)
	at org.jenkinsci.test.acceptance.plugins.ssh_slaves.SshSlaveLauncher.addCredential(SshSlaveLauncher.java:39)
	at org.jenkinsci.test.acceptance.plugins.ssh_slaves.SshSlaveLauncher.pwdCredentials(SshSlaveLauncher.java:69)

In this case I think it would be relatively easy to support both cases with some sort of if/else or try/catch statement, so I think we should strive to preserve compatibility. See #1690 and #1693 for plenty of examples of how this was done before.

Sure thing, updated.

@basil basil changed the title ATH for Credentials Plugin #551 Forward compatibility with jenkinsci/credentials-plugin#551 Oct 6, 2024
@basil basil enabled auto-merge (squash) October 6, 2024 20:36
@basil basil disabled auto-merge October 6, 2024 20:36
@basil basil merged commit 1af10f4 into jenkinsci:master Oct 6, 2024
20 of 24 checks passed
@janfaracik janfaracik deleted the update-dropdown-button branch October 7, 2024 08:00
@basil
Copy link
Member

basil commented Oct 8, 2024

@janfaracik I'm getting the same java.lang.IndexOutOfBoundsException: Index 1 out of bounds for length 1 error on 2.480 in #1771

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants