Adapt plugin for Data Table API 2.0.x

mPokornyETM commented 6 months ago

fixes #634

Do not mixed Jenkins and data-table css classes.

closes #647

Testing done

Tested also with dark mode and it looks fine.

Proposed upgrade guidelines

N/A

Localizations

N/A

Submitter checklist

[x] The Jira / Github issue, if it exists, is well-described.
[x] 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).
- The changelog generator for plugins uses the pull request title as the changelog entry.
- Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during the upgrade.

basil commented 6 months ago

Hey @mpokornyetm, thanks so much for getting this plugin working with the new version of Data Tables. Unfortunately, those two tests are still failing when running in Plugin Compatibility Tester (PCT). I can reproduce the same failures in this repository with

diff --git a/pom.xml b/pom.xml
index b9cd6a3..e77cae1 100644
--- a/pom.xml
+++ b/pom.xml
@@ -70,8 +70,8 @@
     <dependencies>
       <dependency>
     <groupId>io.jenkins.tools.bom</groupId>
-   <artifactId>bom-2.387.x</artifactId>
-   <version>2543.vfb_1a_5fb_9496d</version>
+   <artifactId>bom-2.440.x</artifactId>
+   <version>2950.va_633b_f42f759</version>
     <type>pom</type>
     <scope>import</scope>
       </dependency>

and running:

mvn clean verify -Dtest=org.jenkins.plugins.lockableresources.LockableResourceApiTest#reserveUnreserveApi
mvn clean verify -Dtest=org.jenkins.plugins.lockableresources.LockStepTest#unlockButtonWithWaitingRuns

LockableResourceApiTest#reserveUnreserveApi trips a test assertion:

java.lang.AssertionError: 

Expected: is <true>
     but: was <false>
    at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
    at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
    at org.jenkins.plugins.lockableresources.LockableResourceApiTest.reserveUnreserveApi(LockableResourceApiTest.java:41)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:656)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
    at java.base/java.lang.Thread.run(Thread.java:1583)

org.jenkins.plugins.lockableresources.LockStepTest#unlockButtonWithWaitingRuns simply hangs.

These problems persisted with the latest release of HtmlUnit (4.0.0).

Shortly before these problems occur, Rhino prints JavaScript problems with bootstrap:

Exception class=[org.htmlunit.corejs.javascript.EvaluatorException]
org.htmlunit.ScriptException: missing ( before function parameters. (http://localhost:35677/jenkins/static/f1c98be7/plugin/bootstrap5-api/js/bootstrap.min.js#6)
    at org.htmlunit.javascript.JavaScriptEngine$HtmlUnitContextAction.run(JavaScriptEngine.java:963)
    at org.htmlunit.corejs.javascript.Context.call(Context.java:581)
    at org.htmlunit.corejs.javascript.ContextFactory.call(ContextFactory.java:481)
    at org.htmlunit.javascript.HtmlUnitContextFactory.callSecured(HtmlUnitContextFactory.java:313)
    at org.htmlunit.javascript.JavaScriptEngine.compile(JavaScriptEngine.java:735)
    at org.htmlunit.javascript.JavaScriptEngine.compile(JavaScriptEngine.java:110)
    at org.htmlunit.html.HtmlPage.loadJavaScriptFromUrl(HtmlPage.java:1124)
    at org.htmlunit.html.HtmlPage.loadExternalJavaScriptFile(HtmlPage.java:1015)
    at org.htmlunit.html.ScriptElementSupport.executeScriptIfNeeded(ScriptElementSupport.java:187)
    at org.htmlunit.html.ScriptElementSupport$1.execute(ScriptElementSupport.java:111)
    at org.htmlunit.html.ScriptElementSupport.onAllChildrenAddedToPage(ScriptElementSupport.java:134)
    at org.htmlunit.html.HtmlScript.onAllChildrenAddedToPage(HtmlScript.java:192)
    at org.htmlunit.html.parser.neko.HtmlUnitNekoDOMBuilder.endElement(HtmlUnitNekoDOMBuilder.java:508)
    at org.htmlunit.cyberneko.xerces.parsers.AbstractSAXParser.endElement(AbstractSAXParser.java:303)
    at org.htmlunit.html.parser.neko.HtmlUnitNekoDOMBuilder.endElement(HtmlUnitNekoDOMBuilder.java:454)
    at org.htmlunit.cyberneko.HTMLTagBalancer.callEndElement(HTMLTagBalancer.java:1186)
    at org.htmlunit.cyberneko.HTMLTagBalancer.endElement(HTMLTagBalancer.java:1130)
    at org.htmlunit.cyberneko.filters.DefaultFilter.endElement(DefaultFilter.java:168)
    at org.htmlunit.cyberneko.filters.NamespaceBinder.endElement(NamespaceBinder.java:266)
    at org.htmlunit.cyberneko.HTMLScanner$ContentScanner.scanEndElement(HTMLScanner.java:3362)
    at org.htmlunit.cyberneko.HTMLScanner$ContentScanner.scan(HTMLScanner.java:2055)
    at org.htmlunit.cyberneko.HTMLScanner.scanDocument(HTMLScanner.java:847)
    at org.htmlunit.cyberneko.HTMLConfiguration.parse(HTMLConfiguration.java:336)
    at org.htmlunit.cyberneko.HTMLConfiguration.parse(HTMLConfiguration.java:294)
    at org.htmlunit.cyberneko.xerces.parsers.XMLParser.parse(XMLParser.java:70)
    at org.htmlunit.html.parser.neko.HtmlUnitNekoDOMBuilder.parse(HtmlUnitNekoDOMBuilder.java:750)
    at org.htmlunit.html.parser.neko.HtmlUnitNekoHtmlParser.parse(HtmlUnitNekoHtmlParser.java:199)
    at org.htmlunit.DefaultPageCreator.createHtmlPage(DefaultPageCreator.java:307)
    at org.htmlunit.DefaultPageCreator.createPage(DefaultPageCreator.java:226)
    at org.jvnet.hudson.test.HudsonPageCreator.createPage(HudsonPageCreator.java:53)
    at org.htmlunit.WebClient.loadWebResponseInto(WebClient.java:638)
    at org.htmlunit.WebClient.loadWebResponseInto(WebClient.java:541)
    at org.htmlunit.WebClient.getPage(WebClient.java:459)
    at org.htmlunit.WebClient.getPage(WebClient.java:366)
    at org.htmlunit.WebClient.getPage(WebClient.java:504)
    at org.htmlunit.WebClient.getPage(WebClient.java:486)
    at org.jvnet.hudson.test.JenkinsRule$WebClient.goTo(JenkinsRule.java:2722)
    at org.jvnet.hudson.test.JenkinsRule$WebClient.goTo(JenkinsRule.java:2702)
    at org.jenkins.plugins.lockableresources.TestHelpers.clickButton(TestHelpers.java:78)
    at org.jenkins.plugins.lockableresources.LockStepTest.unlockButtonWithWaitingRuns(LockStepTest.java:764)
    at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
    at java.base/java.lang.reflect.Method.invoke(Method.java:580)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.jvnet.hudson.test.JenkinsRule$1.evaluate(JenkinsRule.java:656)
    at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:299)
    at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:293)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
    at java.base/java.lang.Thread.run(Thread.java:1583)

These problems persist after disabling JavaScript:

diff --git a/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java b/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java
index 4972e7d..13edf41 100644
--- a/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java
+++ b/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java
@@ -746,6 +746,7 @@ public class LockStepTest extends LockStepTestBase {
         p.setDefinition(new CpsFlowDefinition("lock('resource1') { semaphore('wait-inside') }", true));

         JenkinsRule.WebClient wc = j.createWebClient();
+        wc.getOptions().setThrowExceptionOnScriptError(false);

         WorkflowRun prevBuild = null;
         for (int i = 0; i < 3; i++) {
diff --git a/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceApiTest.java b/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceApiTest.java
index 3a7a126..4a70743 100644
--- a/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceApiTest.java
+++ b/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceApiTest.java
@@ -36,6 +36,7 @@ public class LockableResourceApiTest {
         j.jenkins.setAuthorizationStrategy(new FullControlOnceLoggedInAuthorizationStrategy());

         JenkinsRule.WebClient wc = j.createWebClient();
+        wc.getOptions().setThrowExceptionOnScriptError(false);
         wc.login("user");
         TestHelpers.clickButton(wc, "reserve");
         assertThat(LockableResourcesManager.get().fromName("a1").isReserved(), is(true));

This makes no difference.

Per the HtmlUnit maintainer in https://github.com/HtmlUnit/htmlunit/issues/498#issuecomment-1216160780:

This will not work at the moment because some deficits of the current js support. […] some js constructs used in the page code that are not supported at the moment. I have some plans to improve the support in the future.

This message was written on August 15, 2022.

I think you have a few options:

Remove use of fancy JavaScript features and continue to test with HtmlUnit.
Replace HtmlUnit tests with Selenium-based functional tests in the ATH repository (the common pattern for Selenium tests in the Jenkins project) or in this repository (similar to how Dr Hafner's plugins feature Selenium tests in specific plugin repositories)
Disable these HtmlUnit tests temporarily and hope that HtmlUnit eventually supports this fancy JavaScript in the future.
Abandon automated frontend testing permanently and rely on manual testing.

None of these options seem particularly great, but I believe we should choose a path forward one way or another.

uhafner commented 6 months ago

Can you add the explicit version https://github.com/jenkinsci/data-tables-api-plugin/releases/tag/v2.0.3-1 here:

https://github.com/jenkinsci/lockable-resources-plugin/blob/67c1000692dd596babfbe1b4028e0a66cded351c/pom.xml#L89

Then we see if there are test failures.

mPokornyETM commented 5 months ago

Can you add the explicit version https://github.com/jenkinsci/data-tables-api-plugin/releases/tag/v2.0.3-1 here:

https://github.com/jenkinsci/lockable-resources-plugin/blob/67c1000692dd596babfbe1b4028e0a66cded351c/pom.xml#L89

Then we see if there are test failures.

@uhafner it provides some dependency issues see also https://github.com/jenkinsci/lockable-resources-plugin/pull/642/checks?check_run_id=24088006393

uhafner commented 5 months ago

I think you are using a too old BOM that does not match your Jenkins version:

  <properties>
    <changelist>999999-SNAPSHOT</changelist>
    <jenkins.version>2.440.1</jenkins.version>
  </properties>

  <dependencyManagement>
    <dependencies>
      <dependency>
        <groupId>io.jenkins.tools.bom</groupId>
        <artifactId>bom-2.387.x</artifactId>
        <version>2543.vfb_1a_5fb_9496d</version>
        <type>pom</type>
        <scope>import</scope>
      </dependency>
    </dependencies>
  </dependencyManagement>

You should use: <jenkins.version>2.426.3</jenkins.version> or <jenkins.version>2.440.3</jenkins.version> and a matching BOM, like <artifactId>bom-2.426.x</artifactId> or <artifactId>bom-2.440.x</artifactId>

mPokornyETM commented 5 months ago

Remove use of fancy JavaScript features and continue to test with HtmlUnit.

Replace HtmlUnit tests with Selenium-based functional tests in the ATH repository (the common pattern for Selenium tests in the Jenkins project) or in this repository (similar to how Dr Hafner's plugins feature Selenium tests in specific plugin repositories)

Disable these HtmlUnit tests temporarily and hope that HtmlUnit eventually supports this fancy JavaScript in the future.

Abandon automated frontend testing permanently and rely on manual testing.

thx for your time @basil . I mont realy happy :-( No one of the option is fast and good.

about java script. This failures are comming from other plugin, therefore I can not remoive it about selenium. Never did it, I have no experience with the tool, Some example or tutorial for dummies will be great disable HtmlUnit tests. Disable is not good, I dont want to lose code coverage on changes. I am still wating for very long time for the update in the HtmlUnit frame. Manual testing is no more option for me. Not on every change like bom updates

basil commented 5 months ago

about java script. This failures are comming from other plugin, therefore I can not remoive it

Agreed; this option is tantamount to dropping your dependency on the other plugins that use advanced JavaScript.

about selenium. Never did it, I have no experience with the tool, Some example or tutorial for dummies will be great

@uhafner is probably the best person to talk about how to run this from a Jenkins plugin repository since I don't think anyone else does this. For writing tests within the ATH repository there is plenty of documentation at https://github.com/jenkinsci/acceptance-test-harness.

disable HtmlUnit tests. Disable is not good, I dont want to lose code coverage on changes. I am still wating for very long time for the update in the HtmlUnit frame.

Unfortunately, HtmlUnit is increasingly unsuitable for modern JavaScript: https://github.com/HtmlUnit/htmlunit/issues/755 https://github.com/jenkinsci/jenkins-test-harness/issues/569

This would be my preferred short-term approach, and is consistent with the status quo, although it is not great from a testing perspective.

Manual testing is no more option for me. Not on every change like bom updates

For what it's worth, BOM updates of things rarely break frontends. The recent incompatible release of Data Tables is an exception to what is otherwise a fairly consistent track record of compatibility.

jenkinsci / lockable-resources-plugin