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

[JENKINS-43795] Fix not existing Jenkins Test Harness version used #25

Closed
wants to merge 1 commit into from

Conversation

varyvol
Copy link

@varyvol varyvol commented Apr 24, 2017

JENKINS-43795

Some plugins are using jenkins.version to set the jenkins-test-harness.version, which can cause problems when overriding the Jenkins version in PCT as not all versions has a correspondent JTH version.

@reviewbybees esp. @andresrc @kwhetstone @jglick @raul-arabaolaza

String log = IOUtils.toString(is);

if (log.contains("Failure to find org.jenkins-ci.main:jenkins-test-harness")) {
pom.replaceDependencyVersion("org.jenkins-ci.main", "jenkins-test-harness", "RELEASE");
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know using RELEASE is not always appropriate but since this is done in case there is already a failure, I don't think it's a bad thing to use the latest released version.

Anyway any suggestion is more than welcome!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe instead of modifying the pom we can use the jenkins.test-harness.version property?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After private talk with @varyvol 🐝

Copy link
Author

@varyvol varyvol Apr 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify in case someone could have the same doubt: that would not fix the issue in case the plugin explicitly had the dependency without using jenkins-test-harness.version property.

<dependency>
    <artifactId>jenkins-test-harness</artifactId>
    <version>${jenkins.version}</version>
</dependency>

@ghost
Copy link

ghost commented Apr 24, 2017

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@recena
Copy link
Contributor

recena commented Apr 24, 2017

@reviewbybees done

@kwhetstone
Copy link
Contributor

I'd like if this could be caught before hitting an exception. For example, using a preexecution hook to check the jenkins.test.harness version and assign the old coreCoordinates version. That would at least keep the test-harness-version at the level the plugin expects.

@varyvol
Copy link
Author

varyvol commented Apr 24, 2017

@kwhetstone I examined that direction, but how can I know that specific JTH version does not exist? I would need to call an extra command as dependency:resolve for example, so that's why I preferred to use the execution one.

What do you mean with the level the plugin expects?

@kwhetstone
Copy link
Contributor

I'm operating under the assumption that the plugin is able to compile, which should have already happened at preexecution hook time. Maven is already being called in a few different areas; I don't think it'll hurt to call it again. If you really don't want to call it again, and since you're already in MavenPom, I'd add a condition to change only if the version contains "${}" as well as including it within the TransformPom hook.

@jglick
Copy link
Member

jglick commented Apr 24, 2017

Some plugins are using jenkins.version to set the jenkins-test-harness.version

Can you give an example? Why not just fix those plugins?

@varyvol
Copy link
Author

varyvol commented Apr 25, 2017

@jglick dashboard-view 2.9.10 or wikitext 3.7 for example.

@kwhetstone hurting is subjective, but executing one extra call per plugin is quite a big amount of extra time for a complete PCT I think. That's why I thought in doing it just in case of failure. Regarding the conditions:

  • The version is not always declared in the POM itself, but in the parent one.
  • What do you think that should be included within the hook?

@daniel-beck
Copy link
Member

daniel-beck commented Apr 25, 2017

wikitext 3.7

Closed-source commercial plugins should not inform development of Jenkins project supporting infrastructure like PCT.

@jglick
Copy link
Member

jglick commented Apr 25, 2017

As of jenkinsci/dashboard-view-plugin#38 dashboard-view just has

<jenkins.version>1.554.1</jenkins.version>

which is invalid. It should also specify

<jenkins-test-harness.version>1.554.1</jenkins-test-harness.version>

as JTH 2.x is only compatible with 1.580.1+.

@varyvol
Copy link
Author

varyvol commented Apr 26, 2017

Seems like the correct approach would be to analyse and fix the affected plugins. Closing this.

@varyvol varyvol closed this Apr 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants