Config round trip failures in plugin unit tests?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Config round trip failures in plugin unit tests?

Mark Waite-2
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. 

A minimal example of the failure can be seen in https://ci.jenkins.io/job/Plugins/job/git-plugin/view/change-requests/job/PR-1005/2/testReport/ and will also be visible in https://ci.jenkins.io/job/Plugins/job/git-plugin/view/change-requests/job/PR-1005/3/testReport/

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

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/46135ec9-8934-4daf-85b9-909d9e3b8305n%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Config round trip failures in plugin unit tests?

Gavin Mogan
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

On Wed, Nov 25, 2020 at 9:19 PM Mark Waite <[hidden email]> wrote:
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

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/46135ec9-8934-4daf-85b9-909d9e3b8305n%40googlegroups.com.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAG%3D_DusABYiCEhxsV%2Bg653Qg8TYLicQ0v%3DE6hpwOSuxVRuxAUQ%40mail.gmail.com.
Reply | Threaded
Open this post in threaded view
|

Re: Config round trip failures in plugin unit tests?

Daniel Beck-2

On Thu, Nov 26, 2020 at 6:58 AM 'Gavin Mogan' via Jenkins Developers <[hidden email]> 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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAMo7PtLUH63vtAA-fijMDhtQnOy0-vBFR1wKVAY5jP%3Dqse9qNw%40mail.gmail.com.
Reply | Threaded
Open this post in threaded view
|

Re: Config round trip failures in plugin unit tests?

Björn Pedersen
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)

[hidden email] 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 <[hidden email]> 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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/d9f97335-83e6-4eba-ae9c-4139b9e09394n%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Config round trip failures in plugin unit tests?

Mark Waite-2

On Thu, Nov 26, 2020 at 3:17 AM 'Björn Pedersen' via Jenkins Developers <[hidden email]> 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
 
[hidden email] 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 <[hidden email]> 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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/d9f97335-83e6-4eba-ae9c-4139b9e09394n%40googlegroups.com.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAO49JtHAj1A4dO8YR9WSAkxzsS7TvERz_bC_UmK65tGnUL8FVg%40mail.gmail.com.
Reply | Threaded
Open this post in threaded view
|

Re: Config round trip failures in plugin unit tests?

Mark Waite-2
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.
On Thursday, November 26, 2020 at 7:17:57 AM UTC-7 Mark Waite wrote:
On Thu, Nov 26, 2020 at 3:17 AM 'Björn Pedersen' via Jenkins Developers <[hidden email]> 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
 
[hidden email] 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 <[hidden email]> 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 [hidden email].

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/7b767af5-bacb-4a30-9dba-27a4e9b3af73n%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Config round trip failures in plugin unit tests?

Daniel Beck
In reply to this post by Mark Waite-2


> On 28. Nov 2020, at 20:58, Mark Waite <[hidden email]> 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.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/2A8105CE-CBEE-4886-8276-CC4D50528645%40beckweb.net.
Reply | Threaded
Open this post in threaded view
|

Re: Config round trip failures in plugin unit tests?

timja...@gmail.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

On Sat, 28 Nov 2020 at 21:55, Daniel Beck <[hidden email]> wrote:


> On 28. Nov 2020, at 20:58, Mark Waite <[hidden email]> 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.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/2A8105CE-CBEE-4886-8276-CC4D50528645%40beckweb.net.

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAH-3BicjksTx7xMrB%2BVmHhrBDr6pF2OU-XyK%3DxgDSh7NxueSew%40mail.gmail.com.
Reply | Threaded
Open this post in threaded view
|

Re: Config round trip failures in plugin unit tests?

Mark Waite-2

On Sun, Nov 29, 2020 at 1:11 AM Tim Jacomb <[hidden email]> 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
 

--
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 [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAO49JtHZT6v%2Bjqo6LZLfxSWn_p-%2B0pnhV0DMfQ-dt13q5FsiMA%40mail.gmail.com.