Config round trip failures in plugin unit tests?

49 views
Skip to first unread message

Mark Waite

unread,
Nov 26, 2020, 12:19:34 AM11/26/20
to Jenkins Developers
I'm trying to update the git plugin from requiring Jenkins 2.204.1 to Jenkins 2.222.x or newer.  The git plugin includes config round trip tests.  Those tests fail with a message:


The failure line matches a line in the hudson-behavior.js file for that Jenkins core version. 


The second set of failures adds an example that does not use any git plugin features.  Source code for the failing test is available at https://github.com/jenkinsci/git-plugin/pull/1005/files

Suggestions for the changes I should make so that the config round trip tests will run as expected?

Thanks
Mark Waite

Gavin Mogan

unread,
Nov 26, 2020, 12:58:13 AM11/26/20
to Jenkins Developers
I don't know which hasClassName is undefined as your linking to a local file, and https://github.com/jenkinsci/jenkins/blob/master/war/src/main/webapp/scripts/hudson-behavior.js#L1382 doesn't have that function being called.

As a side note, its probably to run `s/Element.hasClassName\((\w+),/$1.classList.contains/g` on that file and get rid of a bit more prototype

Gavin

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/46135ec9-8934-4daf-85b9-909d9e3b8305n%40googlegroups.com.

Daniel Beck

unread,
Nov 26, 2020, 3:50:42 AM11/26/20
to JenkinsCI Developers
On Thu, Nov 26, 2020 at 6:58 AM 'Gavin Mogan' via Jenkins Developers <jenkin...@googlegroups.com> wrote:
I don't know which hasClassName is undefined as your linking to a local file, and https://github.com/jenkinsci/jenkins/blob/master/war/src/main/webapp/scripts/hudson-behavior.js#L1382 doesn't have that function being called.

Björn Pedersen

unread,
Nov 26, 2020, 5:17:15 AM11/26/20
to Jenkins Developers
It looks like the first iteration did not yield any optional block (in master this code is at L1513ff.) and the code never checks that (so s==vg is undefined)

Mark Waite

unread,
Nov 26, 2020, 9:17:57 AM11/26/20
to jenkinsci-dev
On Thu, Nov 26, 2020 at 3:17 AM 'Björn Pedersen' via Jenkins Developers <jenkin...@googlegroups.com> wrote:
It looks like the first iteration did not yield any optional block (in master this code is at L1513ff.) and the code never checks that (so s==vg is undefined)


Thanks for that pointer.  The code that is reporting that error seems to be unchanged (though in a different location in the file) from 2.204.1 through weekly 2.268.

My trivial test runs successfully with a plugin that I generated from the global configuration archetype and runs successfully in the git plugin on the master branch yet it fails on the pull request.  I'll continue exploring to try to understand the difference between the working and non-working cases.

Mark Waite
 
db...@cloudbees.com schrieb am Donnerstag, 26. November 2020 um 09:50:42 UTC+1:
On Thu, Nov 26, 2020 at 6:58 AM 'Gavin Mogan' via Jenkins Developers <jenkin...@googlegroups.com> wrote:
I don't know which hasClassName is undefined as your linking to a local file, and https://github.com/jenkinsci/jenkins/blob/master/war/src/main/webapp/scripts/hudson-behavior.js#L1382 doesn't have that function being called.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.

Mark Waite

unread,
Nov 28, 2020, 2:58:39 PM11/28/20
to Jenkins Developers
I used the global configuration archetype to confirm that the issue is in the dependencies used by the git plugin.  The configuration round trip tests in the global configuration archetype pass when I run them without the git plugin dependencies and they fail when I run them with the git plugin dependencies.

The Apache commons-validator that the git plugin is using as a convenience provider to check remote URL's requires commons-beanutils 1.9.4.  When I include commons-beanutils 1.9.4 as a dependency in the global configuration archetype pom file, the configuration round trip test fails with the TypeError: Cannot call method "hasClassName" message.  When I include commons-beanutils 1.9.3 without the commons-validator, the same tests pass.

Since the commons-validator is just a convenience, I'll remove the dependency from the git plugin and use other means to check git repository URL validity.

I assume  commons-beanutils 1.9.4 (released in August 2019) is not being used by anyone else in Jenkins core or plugins.

Removing commons-validator will be an improvement for the git plugin, since it will remove a transitive dependency on several other components.

Daniel Beck

unread,
Nov 28, 2020, 4:55:29 PM11/28/20
to jenkinsci-dev


> On 28. Nov 2020, at 20:58, Mark Waite <mark.ea...@gmail.com> wrote:
>
> I assume commons-beanutils 1.9.4 (released in August 2019) is not being used by anyone else in Jenkins core or plugins.

commons-beanutils 1.9.4 is known to core maintainers to not be useable in Jenkins. We've now had two PRs for that update, both failed.

Tim Jacomb

unread,
Nov 29, 2020, 3:11:53 AM11/29/20
to jenkin...@googlegroups.com
For the record commons-beanutils is provided by core at version 1.9.3 and it’s in the core bom.

If it’s a transitive dep I would have thought the bom would have corrected the version, otherwise you can declare the dep without a version and scope provided and the bom should handle it

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.

Mark Waite

unread,
Nov 29, 2020, 8:51:58 AM11/29/20
to jenkinsci-dev
On Sun, Nov 29, 2020 at 1:11 AM Tim Jacomb <timja...@gmail.com> wrote:
For the record commons-beanutils is provided by core at version 1.9.3 and it’s in the core bom.

If it’s a transitive dep I would have thought the bom would have corrected the version, otherwise you can declare the dep without a version and scope provided and the bom should handle it


Thanks for the further insights.  I think I've understood the fundamental mistake I made.

When I switched from requiring Jenkins 2.204.1 to require Jenkins 2.222.4 as the minimum Jenkins version, switched to use the 2.222.x bom, and removed the exclusion of commons-beanutils, the GitStepTest.configRoundTrip automated test consistently failed with a JavaScript error.

I could have continued using commons-validator 1.7 if I'd continued excluding commons-beanutils as a dependency of commons-validator 1.7.  My desire to remove exclusions got in the way.

The same issue would have been visible to me in Jenkins 2.204.1 if I had removed the exclusion of commons-beanutils from the commons-validator dependency.  The root mistake I made was removing the exclusion of commons-beanutils.  Removing that exclusion caused commons-beanutils 1.9.4 to be included in the git.hpi file.  That inclusion broke the tests.  The increase of minimum Jenkins version from 2.204.1 to 2.222.4 was not involved in the issue.

The problem is resolved by https://github.com/jenkinsci/git-plugin/pull/1009 where the dependency on commons-validator is removed.  Validating user input is important, but the checks that were being performed by commons-validator are not so much more valuable that I should wrestle with dependencies for them.

Thanks for the insights!
Mark Waite
 
Reply all
Reply to author
Forward
0 new messages