• Icon: Improvement Improvement
    • Resolution: Fixed
    • Icon: Minor Minor
    • openid-plugin
    • None

      For improved test-ability and upgrades, upgrade the parent pom to the 2.x line. The jenkins.version might also be a candidate for upgrades since it's on a line that's old.

          [JENKINS-36499] Migrate openid to parent pom

          Kristin Whetstone created issue -

          While investigating this conversion, I ran into a lot of issues with how the tests ran. Noteably, there are some tests that I don't think will ever pass. I think these test failures will have to be addressed by the maintainers of the plugin. Also, I think there are some legitimate issues with the plugin since I tried using it on the latest version of Jenkins and ran into 404 errors while trying to use the service.

          Kristin Whetstone added a comment - While investigating this conversion, I ran into a lot of issues with how the tests ran. Noteably, there are some tests that I don't think will ever pass. I think these test failures will have to be addressed by the maintainers of the plugin. Also, I think there are some legitimate issues with the plugin since I tried using it on the latest version of Jenkins and ran into 404 errors while trying to use the service.
          Kristin Whetstone made changes -
          Link New: This issue is related to JENKINS-23075 [ JENKINS-23075 ]

          Also, the tests won't pass on their own regardless of the changes I've made with htmlunit. I think this plugin might need some expert eyes to try to fix some of the functionality on the newest versions.

          Kristin Whetstone added a comment - Also, the tests won't pass on their own regardless of the changes I've made with htmlunit. I think this plugin might need some expert eyes to try to fix some of the functionality on the newest versions.

          Ran into this when attempting to run `OpenIdLoginServiceTest`s

          Kristin Whetstone added a comment - Ran into this when attempting to run `OpenIdLoginServiceTest`s
          Kristin Whetstone made changes -
          Link New: This issue is related to JENKINS-24646 [ JENKINS-24646 ]

          I've had to disable almost every test while attempting to convert this to use the parent pom. I'm perfectly fine with a simpler changeset that uses an older parent pom if it meant that the tests would also pass. I'm not entirely sure if this is a setup issue on my system, as I have witnessed at least part of the openid service working when configuring it manually in the 1.651.1 version of Jenkins. Some of the exceptions I've seen are a result of the jetty server messing with the trace of openid4java: no crumbIssuer should be configured for these tests while the crumbIssuer can't be set/modified after the server starts. (Somehow, both HudsonTestCase and JenkinsRule both start the server then attempt to change the crumbIssuer while I've only seen explicit errors around the situation while using JenkinsRule).

          This might just be an issue on my local setup.

          Kristin Whetstone added a comment - I've had to disable almost every test while attempting to convert this to use the parent pom. I'm perfectly fine with a simpler changeset that uses an older parent pom if it meant that the tests would also pass. I'm not entirely sure if this is a setup issue on my system, as I have witnessed at least part of the openid service working when configuring it manually in the 1.651.1 version of Jenkins. Some of the exceptions I've seen are a result of the jetty server messing with the trace of openid4java: no crumbIssuer should be configured for these tests while the crumbIssuer can't be set/modified after the server starts. (Somehow, both HudsonTestCase and JenkinsRule both start the server then attempt to change the crumbIssuer while I've only seen explicit errors around the situation while using JenkinsRule). This might just be an issue on my local setup.

          Nope, the problem with the failing test appears when trying to test with the jenkins.ci server from the master branch of the current test suite. The only option in trying to convert all these tests is to:a
          a) ignore all the tests and mark them as something that has to be fixed in the future
          b) accept the changeset with failing tests knowing that the solution has to be found in the future

          Of the 2 options, the 2nd seems more favorable to me, simply from the perspective that the changes go into the codebase.

          The major issue I've seen in the test case conversion is the openid4java throwing errors when the crumbIssuer can't be set after the service has been started. This shows up in old and new tests, and there's really no good way to set up that particular setting. (The jetty server errors on stop/restart and any other overrides aren't a good 1-1 replacement)

          Kristin Whetstone added a comment - Nope, the problem with the failing test appears when trying to test with the jenkins.ci server from the master branch of the current test suite. The only option in trying to convert all these tests is to:a a) ignore all the tests and mark them as something that has to be fixed in the future b) accept the changeset with failing tests knowing that the solution has to be found in the future Of the 2 options, the 2nd seems more favorable to me, simply from the perspective that the changes go into the codebase. The major issue I've seen in the test case conversion is the openid4java throwing errors when the crumbIssuer can't be set after the service has been started. This shows up in old and new tests, and there's really no good way to set up that particular setting. (The jetty server errors on stop/restart and any other overrides aren't a good 1-1 replacement)
          Kristin Whetstone made changes -
          Remote Link New: This issue links to "TestOpenid (Web Link)" [ 14604 ]
          Kristin Whetstone made changes -
          Status Original: Open [ 1 ] New: In Progress [ 3 ]

            kwhetstone Kristin Whetstone
            kwhetstone Kristin Whetstone
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: