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

Rewrite the build history widget #9148

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

janfaracik
Copy link
Contributor

@janfaracik janfaracik commented Apr 9, 2024

This PR implements the changes discussed https://community.jenkins.io/t/revamped-build-history-widget/10944. In essence it's a rewrite/redesign of the build history widget. The goal is to be cleaner and simpler, whilst still retaining existing functionality and density.

Before
image
image
image

After
image

image image image

What's changed

  • The JS has been rewritten and does away with a lot of the complicated layout logic in favour of CSS flex/grid.
  • A new card component has been added (with the intention of replacing the existing widget component) that supports inline actions
  • Builds now group by date, keeping the UI simpler/easier to read
  • RSS feed has been moved to a dropdown, rather than a fixed button row
  • Build overflow menus are now always present on the right of the link

What doesn't work

  • Pagination controls are always visible at the moment
  • The list doesn't auto refresh at the moment
  • Items with long names/lots of actions don't scale very well

Testing done

  • See Rewrite the build history widget #9148 (comment) for a view of the scenarios tested
    • Normal scenarios work as expected (e.g. short build names, one or two badges)
    • Queued items look as expected
    • Items with plain text/HTML descriptions work as expected
    • Items with long names/lots of badges wrap as expected

Proposed changelog entries

  • Update the design of the build history widget

Proposed upgrade guidelines

N/A

Submitter checklist

Edit tasklist title
Beta Give feedback Tasklist Submitter checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. The Jira issue, if it exists, is well-described.
    Options
  2. The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
    Options
  3. There is automated testing or an explanation as to why this change has no tests.
    Options
  4. New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
    Options
  5. New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
    Options
  6. New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
    Options
  7. For dependency updates, there are links to external changelogs and, if possible, full differentials.
    Options
  8. For new APIs and extension points, there is a link to at least one consumer.
    Options

Desired reviewers

@jenkinsci/sig-ux

Before the changes are marked as ready-for-merge:

Maintainer checklist

Edit tasklist title
Beta Give feedback Tasklist Maintainer checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. There are at least two (2) approvals for the pull request and no outstanding requests for change.
    Options
  2. Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
    Options
  3. Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
    Options
  4. Proper changelog labels are set so that the changelog can be generated automatically.
    Options
  5. If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
    Options
  6. If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).
    Options

commit 1ce4cb3
Author: Jan Faracik <[email protected]>
Date:   Tue Apr 9 09:22:36 2024 +0100

    Rename classes

commit 107d794
Author: Jan Faracik <[email protected]>
Date:   Tue Apr 9 08:59:54 2024 +0100

    Update HistoryWidget.java

commit 13575bc
Author: Jan Faracik <[email protected]>
Date:   Tue Apr 9 08:59:29 2024 +0100

    Hide buttons

commit e5de546
Author: Jan Faracik <[email protected]>
Date:   Tue Apr 9 08:39:04 2024 +0100

    Rename classes

commit 47c84af
Author: Jan Faracik <[email protected]>
Date:   Mon Apr 8 23:21:36 2024 +0100

    Add animation

commit e7432b6
Author: Jan Faracik <[email protected]>
Date:   Mon Apr 8 23:03:19 2024 +0100

    Add navigation buttons

commit 448a094
Author: Jan Faracik <[email protected]>
Date:   Mon Apr 8 22:29:32 2024 +0100

    Update _dashboard.scss

commit bb85734
Merge: c451a72 27433f1
Author: Jan Faracik <[email protected]>
Date:   Mon Apr 8 22:23:38 2024 +0100

    Merge branch 'master' into new-build-history-2

commit c451a72
Author: Jan Faracik <[email protected]>
Date:   Mon Apr 8 13:17:05 2024 +0100

    Update _job.scss

commit 960b162
Merge: d020eb6 af655e3
Author: Jan Faracik <[email protected]>
Date:   Mon Apr 8 13:16:54 2024 +0100

    Merge branch 'remove-table-usage' into new-build-history-2

commit af655e3
Author: Jan Faracik <[email protected]>
Date:   Mon Apr 8 13:13:38 2024 +0100

    Init

commit d020eb6
Author: Jan Faracik <[email protected]>
Date:   Mon Apr 8 13:09:57 2024 +0100

    Update card.jelly

commit 81bc1d4
Author: Jan Faracik <[email protected]>
Date:   Mon Apr 8 13:07:41 2024 +0100

    Update _buttons.scss

commit 4023460
Merge: 875fb8f da5f593
Author: Jan Faracik <[email protected]>
Date:   Mon Apr 8 13:03:18 2024 +0100

    Merge branch 'master' into new-build-history-2

commit 875fb8f
Merge: 2dc9964 fe60fac
Author: Jan Faracik <[email protected]>
Date:   Sat Apr 6 19:29:20 2024 +0100

    Merge branch 'master' into new-build-history-2

commit 2dc9964
Author: Jan Faracik <[email protected]>
Date:   Fri Apr 5 22:17:08 2024 +0100

    Reset files

commit 2da1e14
Merge: dce466a b9fac75
Author: Jan Faracik <[email protected]>
Date:   Fri Apr 5 22:14:46 2024 +0100

    Merge branch 'master' into new-build-history-2

commit dce466a
Merge: 935c16e 0eed048
Author: Jan Faracik <[email protected]>
Date:   Wed Feb 28 13:55:35 2024 +0000

    Merge branch 'master' into new-build-history-2

commit 935c16e
Author: Jan Faracik <[email protected]>
Date:   Sun Feb 18 17:54:32 2024 +0000

    Update entry.jelly

commit 2a19a04
Author: Jan Faracik <[email protected]>
Date:   Sun Feb 18 17:51:58 2024 +0000

    Rename classes

commit 3ef46ab
Author: Jan Faracik <[email protected]>
Date:   Sun Feb 18 17:29:25 2024 +0000

    Update index.jelly

commit da0b212
Merge: 281fac0 9d9e2ab
Author: Jan Faracik <[email protected]>
Date:   Sun Feb 18 17:27:12 2024 +0000

    Merge branch 'revamp-dropdowns' into new-build-history-2

commit 281fac0
Merge: 9c69bd0 a642354
Author: Jan Faracik <[email protected]>
Date:   Sun Feb 18 17:19:14 2024 +0000

    Merge branch 'master' into new-build-history-2

commit 9d9e2ab
Author: Jan Faracik <[email protected]>
Date:   Sat Feb 10 19:39:24 2024 +0000

    Update templates.js

commit 1c19e43
Author: Jan Faracik <[email protected]>
Date:   Sat Feb 10 19:38:13 2024 +0000

    Add clazz

commit 8b944e9
Author: Jan Faracik <[email protected]>
Date:   Sat Feb 10 15:19:30 2024 +0000

    Update utils.js

commit 069fefb
Author: Jan Faracik <[email protected]>
Date:   Sat Feb 10 13:23:16 2024 +0000

    Linting

commit 7270712
Merge: 9865811 a642354
Author: Jan Faracik <[email protected]>
Date:   Sat Feb 10 12:20:02 2024 +0000

    Merge branch 'master' into revamp-dropdowns

commit 9865811
Merge: 1e22c34 86d39dd
Author: Mark Waite <[email protected]>
Date:   Thu Jan 18 05:39:13 2024 -0700

    Merge branch 'master' into revamp-dropdowns

commit 9c69bd0
Author: Jan Faracik <[email protected]>
Date:   Sun Jan 14 13:56:49 2024 +0000

    Push

commit 347e966
Author: Jan Faracik <[email protected]>
Date:   Sun Jan 14 13:53:52 2024 +0000

    Update filter-build-history.js

commit 0b4a5dd
Author: Jan Faracik <[email protected]>
Date:   Sun Jan 14 13:49:54 2024 +0000

    Renam

commit a8277bf
Author: Jan Faracik <[email protected]>
Date:   Sun Jan 14 13:46:53 2024 +0000

    Fix

commit 855bf13
Merge: 61b0a87 1eb29a8
Author: Jan Faracik <[email protected]>
Date:   Sun Jan 14 13:38:51 2024 +0000

    Merge branch 'master' into new-build-history-2

commit 1e22c34
Merge: 44981c2 48661db
Author: Jan Faracik <[email protected]>
Date:   Sun Jan 14 13:33:14 2024 +0000

    Merge branch 'master' into revamp-dropdowns

commit 44981c2
Merge: 0075375 1eb29a8
Author: Jan Faracik <[email protected]>
Date:   Mon Jan 8 21:14:51 2024 +0000

    Merge branch 'master' into revamp-dropdowns

commit 0075375
Merge: 2dd9e32 78cdaa9
Author: Jan Faracik <[email protected]>
Date:   Thu Jan 4 13:26:24 2024 +0000

    Merge branch 'master' into revamp-dropdowns

commit 2dd9e32
Author: Jan Faracik <[email protected]>
Date:   Thu Jan 4 13:24:53 2024 +0000

    Remove translations

commit 6800c88
Author: Jan Faracik <[email protected]>
Date:   Thu Jan 4 13:16:19 2024 +0000

    Update header.jelly

commit 1c3961b
Author: Jan Faracik <[email protected]>
Date:   Thu Jan 4 13:15:49 2024 +0000

    Add additional docs

commit 163be52
Merge: 4cc43e4 444f2de
Author: Jan Faracik <[email protected]>
Date:   Wed Jan 3 21:22:20 2024 +0000

    Merge branch 'master' into revamp-dropdowns

commit 61b0a87
Author: Jan Faracik <[email protected]>
Date:   Thu Dec 14 19:38:45 2023 +0000

    More

commit dcd6aaa
Merge: 0c40b9f edce488
Author: Jan Faracik <[email protected]>
Date:   Thu Dec 14 19:38:25 2023 +0000

    Merge branch 'stop-button' into new-build-history-2

commit edce488
Author: Jan Faracik <[email protected]>
Date:   Thu Dec 14 16:06:00 2023 +0000

    Tidy up

commit 157ba0b
Author: Jan Faracik <[email protected]>
Date:   Thu Dec 14 16:04:46 2023 +0000

    Fix i18n

commit a112bd9
Author: Jan Faracik <[email protected]>
Date:   Wed Dec 13 22:57:17 2023 +0000

    Update _buttons.scss

commit 91751cf
Author: Jan Faracik <[email protected]>
Date:   Wed Dec 13 22:39:18 2023 +0000

    Update executors.jelly

commit cd89aea
Author: Jan Faracik <[email protected]>
Date:   Wed Dec 13 22:31:06 2023 +0000

    Fixes

commit 1384091
Author: Jan Faracik <[email protected]>
Date:   Wed Dec 13 22:28:54 2023 +0000

    Init

commit 0c40b9f
Author: Jan Faracik <[email protected]>
Date:   Wed Dec 13 16:50:53 2023 +0000

    Update _buttons.scss

commit 50fe8fc
Author: Jan Faracik <[email protected]>
Date:   Wed Dec 13 13:13:10 2023 +0000

    add view transitions

commit d1fd7a9
Author: Jan Faracik <[email protected]>
Date:   Tue Dec 12 20:25:44 2023 +0000

    Tidy up

commit 512b9a1
Author: Jan Faracik <[email protected]>
Date:   Tue Dec 12 19:29:16 2023 +0000

    Push

commit 4f15690
Author: Jan Faracik <[email protected]>
Date:   Mon Dec 11 23:30:09 2023 +0000

    push

commit ad75b0f
Author: Jan Faracik <[email protected]>
Date:   Mon Dec 11 23:24:11 2023 +0000

    Update _buttons.scss

commit cec222e
Author: Jan Faracik <[email protected]>
Date:   Mon Dec 11 23:23:52 2023 +0000

    Update _buttons.scss

commit 4cc43e4
Author: Jan Faracik <[email protected]>
Date:   Sun Dec 10 16:04:28 2023 +0000

    Add docs

commit a4c7f4f
Author: Jan Faracik <[email protected]>
Date:   Sun Dec 10 16:00:23 2023 +0000

    Update taglib

commit c01db44
Author: Jan Faracik <[email protected]>
Date:   Sun Dec 10 15:56:50 2023 +0000

    Init

commit 21bb3f4
Merge: d3e2920 428d0e5
Author: Jan Faracik <[email protected]>
Date:   Sun Dec 10 13:22:00 2023 +0000

    Merge branch 'restyle-cards' into new-build-history-2

commit 428d0e5
Author: Jan Faracik <[email protected]>
Date:   Thu Dec 7 20:17:10 2023 +0000

    Lower weight

commit ac5c255
Author: Jan Faracik <[email protected]>
Date:   Thu Dec 7 20:15:28 2023 +0000

    Remove more bold weights

commit 2922a69
Author: Jan Faracik <[email protected]>
Date:   Thu Dec 7 20:11:33 2023 +0000

    Update _style.scss

commit 9657d46
Author: Jan Faracik <[email protected]>
Date:   Thu Dec 7 20:06:16 2023 +0000

    Init

commit d3e2920
Merge: a2bd9a0 cabc8f6
Author: Jan Faracik <[email protected]>
Date:   Thu Dec 7 19:50:43 2023 +0000

    Merge branch 'master' into new-build-history-2

commit a2bd9a0
Author: Jan Faracik <[email protected]>
Date:   Sun Nov 26 11:58:22 2023 +0000

    Fixes

commit 7cda504
Author: Jan Faracik <[email protected]>
Date:   Sat Nov 25 12:28:04 2023 +0000

    Working build

commit abd994e
Author: Jan Faracik <[email protected]>
Date:   Sat Nov 25 11:58:42 2023 +0000

    Working build

commit 9b6defb
Merge: 29fcb64 7a0e57e
Author: Jan Faracik <[email protected]>
Date:   Sat Nov 25 11:51:24 2023 +0000

    Merge branch 'progress-bar-new' into new-build-history-2

commit 29fcb64
Author: Jan Faracik <[email protected]>
Date:   Sat Nov 25 11:51:10 2023 +0000

    More

commit 2f946d3
Author: Jan Faracik <[email protected]>
Date:   Sat Nov 25 09:17:06 2023 +0000

    Update _buttons.scss

commit f5474b3
Merge: 00c3879 982bc48
Author: Jan Faracik <[email protected]>
Date:   Sat Nov 25 09:16:14 2023 +0000

    Merge branch 'use-symbols-for-build-status-new' into new-build-history-2

commit 982bc48
Author: Jan Faracik <[email protected]>
Date:   Sat Nov 25 09:15:56 2023 +0000

    Fix app bar build status icon being incorrect

commit 00c3879
Author: Jan Faracik <[email protected]>
Date:   Sat Nov 25 09:12:10 2023 +0000

    Fixes

commit a4960e9
Merge: d28aada c6f5db0
Author: Jan Faracik <[email protected]>
Date:   Sat Nov 25 08:56:01 2023 +0000

    Merge branch 'use-symbols-for-build-status-new' into new-build-history-2

commit d28aada
Author: Jan Faracik <[email protected]>
Date:   Sat Nov 25 08:55:18 2023 +0000

    Update _buttons.scss

commit 1d24a19
Author: Jan Faracik <[email protected]>
Date:   Sat Nov 25 08:52:58 2023 +0000

    Init

commit 7a0e57e
Author: Jan Faracik <[email protected]>
Date:   Fri Nov 24 21:19:23 2023 +0000

    More

commit 67d4264
Author: Jan Faracik <[email protected]>
Date:   Fri Nov 24 21:17:09 2023 +0000

    Update _spinner.scss

commit 9befc76
Author: Jan Faracik <[email protected]>
Date:   Fri Nov 24 21:07:55 2023 +0000

    Update _spinner.scss

commit 528b46a
Author: Jan Faracik <[email protected]>
Date:   Fri Nov 24 21:01:06 2023 +0000

    More

commit ea0c487
Author: Jan Faracik <[email protected]>
Date:   Fri Nov 24 20:36:53 2023 +0000

    Init

commit c6f5db0
Author: Jan Faracik <[email protected]>
Date:   Fri Nov 24 17:52:28 2023 +0000

    Fix icon position

commit 18a8407
Merge: aea4d97 a9c34d7
Author: Jan Faracik <[email protected]>
Date:   Fri Nov 24 09:58:54 2023 +0000

    Merge branch 'master' into use-symbols-for-build-status-new

commit aea4d97
Author: Jan Faracik <[email protected]>
Date:   Fri Nov 24 09:58:29 2023 +0000

    Rename ID

commit 5f76f38
Author: Jan Faracik <[email protected]>
Date:   Mon Nov 20 16:00:14 2023 +0000

    Init
@daniel-beck daniel-beck marked this pull request as draft April 9, 2024 09:17
@timja
Copy link
Member

timja commented Apr 9, 2024

can you add before screenshots too please

@timja timja added rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted web-ui The PR includes WebUI changes which may need special expertise labels Apr 9, 2024
@mawinter69
Copy link
Contributor

How does this look when a build has a display name?
How does it look when a build has many badges?
Just an example how this looks currently:
image

@janfaracik
Copy link
Contributor Author

How does this look when a build has a display name? How does it look when a build has many badges? Just an example how this looks currently: image

image

It's okay but could be better - I think moving the badges onto a new row when its incredibly long would improve it.

@mawinter69
Copy link
Contributor

What made the previous implementation so big was the logic to find a suitable way to arrange all the things. See #9122 for my PR that changed they calculation to be more efficient.

@github-actions github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Apr 10, 2024
Copy link

Please take a moment and address the merge conflicts of your pull request. Thanks!

@github-actions github-actions bot removed the unresolved-merge-conflict There is a merge conflict with the target branch. label Apr 10, 2024
@janfaracik
Copy link
Contributor Author

Getting ReferenceError: "fetch" is not defined. (http://localhost:42617/jenkins/static/8bd93229/jsbundles/pages/project/builds-card.js#52) in tests - have you experienced this before @timja?

@timja
Copy link
Member

timja commented Apr 11, 2024

Getting ReferenceError: "fetch" is not defined. (http://localhost:42617/jenkins/static/8bd93229/jsbundles/pages/project/builds-card.js#52) in tests - have you experienced this before @timja?

I see the issue, PR incoming

@timja
Copy link
Member

timja commented Apr 11, 2024

jenkinsci/jenkins-test-harness#753

@timja timja requested a review from a team April 15, 2024 12:07
@timja timja added the needs-security-review Awaiting review by a security team member label Apr 15, 2024
@Kevin-CB Kevin-CB added security-approved @jenkinsci/core-security-review reviewed this PR for security issues and removed needs-security-review Awaiting review by a security team member labels Apr 23, 2024
@Kevin-CB
Copy link
Contributor

No blockers from a security perspective.

However, I've noticed that the search filter doesn't seem to work properly.
It takes quite long or never finishes.

@github-actions github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Apr 23, 2024
Copy link

Please take a moment and address the merge conflicts of your pull request. Thanks!

@github-actions github-actions bot removed the unresolved-merge-conflict There is a merge conflict with the target branch. label Apr 23, 2024
@janfaracik
Copy link
Contributor Author

StaticRoutingDecisionProviderTest#test_objectCustom_withStandardWhitelist is passing locally but not on CI - wondering if this is actually an issue of this PR?

@mawinter69
Copy link
Contributor

mawinter69 commented Apr 25, 2024

The collapse arrow is no longer there.
tbh I have never used it so I'm not missing it but don't know what others are thinking about it.

@daniel-beck
Copy link
Member

StaticRoutingDecisionProviderTest#test_objectCustom_withStandardWhitelist is passing locally but not on CI - wondering if this is actually an issue of this PR?

I can reproduce it locally by running the entire test class, not just the one test. This is a result of concurrent test execution within the same class. I was unaware we were doing this, is this new?

@timja
Copy link
Member

timja commented Apr 29, 2024

StaticRoutingDecisionProviderTest#test_objectCustom_withStandardWhitelist is passing locally but not on CI - wondering if this is actually an issue of this PR?

I can reproduce it locally by running the entire test class, not just the one test. This is a result of concurrent test execution within the same class. I was unaware we were doing this, is this new?

I can't see it enabled =/

jenkins/test/pom.xml

Lines 359 to 369 in 065c2ba

<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<!-- version specified in grandparent pom -->
<configuration>
<argLine>@{jacocoSurefireArgs} -Xmx1g @{jenkins.addOpens} @{jenkins.insaneHook}</argLine>
<systemPropertyVariables>
<hudson.maven.debug>${mavenDebug}</hudson.maven.debug>
<buildDirectory>${project.build.directory}</buildDirectory>
</systemPropertyVariables>
<reuseForks>false</reuseForks>
</configuration>

@daniel-beck
Copy link
Member

OK then I'll dig further, my System.err based diagnostic probably wasn't good enough :)

@daniel-beck
Copy link
Member

Running the entire class, I get this after modifying the test to not rely on @Before and changing the URL to be guaranteed not routable:

Screenshot 2024-04-29 at 11 32 57

@timja Thoughts? Am I missing something obvious?

@timja
Copy link
Member

timja commented Apr 29, 2024

I've not looked into it in detail but this chases it away =/:

diff --git a/test/src/test/java/jenkins/security/stapler/StaticRoutingDecisionProviderTest.java b/test/src/test/java/jenkins/security/stapler/StaticRoutingDecisionProviderTest.java
index 9e849a2278..b76124047d 100644
--- a/test/src/test/java/jenkins/security/stapler/StaticRoutingDecisionProviderTest.java
+++ b/test/src/test/java/jenkins/security/stapler/StaticRoutingDecisionProviderTest.java
@@ -95,7 +95,7 @@ public class StaticRoutingDecisionProviderTest extends StaplerAbstractTest {
     }

     @Test
-    public void test_job_index() throws Exception {
+    public void zzTest_job_index() throws Exception {
         j.createFreeStyleProject("testProject");
         assertReachableWithoutOk("contentProvider/job/");
         assertTrue(ContentProvider.called);

@timja
Copy link
Member

timja commented Apr 29, 2024

Its being loaded by build history ajax somehow.

From a breakpoint on all the called variables:
image

@timja
Copy link
Member

timja commented Apr 29, 2024

This also chases it away, I haven't run other sub-classes to see if any impact:

diff --git a/test/src/test/java/jenkins/security/stapler/StaplerAbstractTest.java b/test/src/test/java/jenkins/security/stapler/StaplerAbstractTest.java
index 4c3f5d28b6..d66902a724 100644
--- a/test/src/test/java/jenkins/security/stapler/StaplerAbstractTest.java
+++ b/test/src/test/java/jenkins/security/stapler/StaplerAbstractTest.java
@@ -43,7 +43,7 @@ import org.htmlunit.HttpMethod;
 import org.htmlunit.Page;
 import org.htmlunit.WebRequest;
 import org.junit.Before;
-import org.junit.ClassRule;
+import org.junit.Rule;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.kohsuke.stapler.Stapler;
 import org.kohsuke.stapler.StaplerResponse;
@@ -51,8 +51,8 @@ import org.kohsuke.stapler.WebApp;
 import org.kohsuke.stapler.WebMethod;

 public abstract class StaplerAbstractTest {
-    @ClassRule
-    public static JenkinsRule rule = new JenkinsRule();
+    @Rule
+    public JenkinsRule rule = new JenkinsRule();
     protected JenkinsRule j;

     protected JenkinsRule.WebClient wc;

@daniel-beck
Copy link
Member

daniel-beck commented Apr 29, 2024

Nice find. Web client reuse seems sus and unnecessary, I'm looking into cleaning that up, hoping this goes away with that.

Making JenkinsRule a @ClassRule probably speeds tests up considerably, would need to look into whether that decision was documented.

@timja
Copy link
Member

timja commented Apr 29, 2024

Making JenkinsRule a @ClassRule probably speeds tests up considerably, would need to look into whether that decision was documented.

Yeah, 3 sec ClassRule, 11 sec Rule

@timja
Copy link
Member

timja commented Apr 30, 2024

nice build passes now, cheers Daniel

@janfaracik
Copy link
Contributor Author

Big thanks both 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted security-approved @jenkinsci/core-security-review reviewed this PR for security issues web-ui The PR includes WebUI changes which may need special expertise
Projects
None yet
5 participants