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

Update pipeline-model-definition and fix related test #2555

Merged
merged 13 commits into from
Apr 10, 2024

Conversation

timja
Copy link
Member

@timja timja commented Apr 10, 2024

Description

see jenkinsci/pipeline-model-definition-plugin#700
(save resources not running against the full bom as its only this plugin it affects)

I've tried running this locally but I can't get the plugin to build due to old npm.
tried on both mac intel and arm.

Submitter checklist

  • Link to JIRA ticket in description, if appropriate.
  • Change is code complete and matches issue description
  • Appropriate unit or acceptance tests or explanation to why this change has no tests
  • Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

  • Run the changes and verified the change matches the issue description
  • Reviewed the code
  • Verified that the appropriate tests have been written or valid explanation given

@dwnusbaum dwnusbaum changed the title Test upcoming declarative pipeline Update pipeline-model-definition and fix related test Apr 10, 2024
@dwnusbaum dwnusbaum changed the title Update pipeline-model-definition and fix related test Update pipeline-model-definition and fix related test Apr 10, 2024
@@ -10,6 +12,7 @@ public class ExportConfig {
* @deprecated
* Use getter and setter
*/
@SuppressFBWarnings(value="PA_PUBLIC_PRIMITIVE_ATTRIBUTE", justification="Preserve API compatibility")
Copy link
Member

Choose a reason for hiding this comment

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

Required because the parent POM update picks up a new SpotBugs version. The justification is just copied over from https://github.com/jenkinsci/stapler/blob/35363c1920d2701aeefb05b57b07d721dfc48c5d/core/src/main/java/org/kohsuke/stapler/export/ExportConfig.java#L16. I do not know why we have a copy of half of that package here.

@@ -514,16 +516,26 @@
<artifactId>handy-uri-templates</artifactId>
<version>2.1.8</version>
</dependency>
<!-- enforcer RequireUpperBoundDeps -->
Copy link
Member

Choose a reason for hiding this comment

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

The two old entries were actually causing upper bounds errors after the Jenkins and BOM updates.

@@ -37,7 +37,7 @@
<groupId>org.eclipse.jgit</groupId>
<artifactId>org.eclipse.jgit.ssh.jsch</artifactId>
<!-- TODO this is hard to manage; if needed, should be in BOM -->
<version>6.6.1.202309021850-r</version>
<version>6.9.0.202403050737-r</version>
Copy link
Member

Choose a reason for hiding this comment

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

Needed to match the BOM updates.

@@ -122,6 +122,7 @@ public Object save(@NonNull String apiUrl, @Nullable String owner, @Nullable Str
*
* @return If new branch is created, sha of content.path on the new branch otherwise null
*/
@SuppressFBWarnings(value = "NP_UNWRITTEN_FIELD", justification = "Fields populated reflectively when deserializing JSON to Java")
Copy link
Member

Choose a reason for hiding this comment

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

Required because the parent POM update picks up a new SpotBugs version.

@dwnusbaum
Copy link
Member

All tests passed besides RunImplTest.pipelineLatestRunIncludesRunning on the Linux JDK 17 branch (it did not fail on the Linux JDK 21 branch). The test passed for me locally as well, so I assume it is just flaky, although the reason for it being flaky is not immediately obvious to me.

@timja timja marked this pull request as ready for review April 10, 2024 20:41
@timja timja requested a review from a team as a code owner April 10, 2024 20:41
@dwnusbaum
Copy link
Member

@timja Maybe better to leave this in in draft state until the release is out.

pom.xml Outdated Show resolved Hide resolved
@olamy olamy merged commit 8eebf88 into jenkinsci:master Apr 10, 2024
13 checks passed
@timja timja deleted the test-pct branch April 11, 2024 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants