[JENKINS-44266, JENKINS-36654] Fix PCT for 2.46.2 #103
Conversation
…st newer core versions (PCT).
|
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. |
There was a problem hiding this comment.
This seems suspicious. There is no mention of subversion (or Subversion or svn) in the repository. So why do tests need this dependency?
When filing issues like JENKINS-44266 it would be appreciated if you would mention what the actual test failures are. Otherwise it is very difficult for reviewers to understand why you are proposing a change.
|
@jglick @oleg-nenashev it's the usual issue with transitive test dependencies. I have improved the JIRA issue with the actual error message. |
|
So does jenkinsci/plugin-compat-tester#28 fix it? |
|
@jglick no, that would avoid a transitive optional dependency to be added to the POM, but this is failing even without modifying the POM (check that the command just changes the core version). |
|
I thought I saw that this plugin was not in the |
|
@oleg-nenashev @jglick do you think this need further work? |
There was a problem hiding this comment.
I suspect what you actually wanted to do was update the parent POM so as to pick up jenkinsci/jenkins-test-harness#54. But this seems harmless anyway.
|
@oleg-nenashev could you review this again? |
There was a problem hiding this comment.
Subversion plugin 2.5.2 requires Jenkins Core 1.568: https://github.com/jenkinsci/subversion-plugin/blob/subversion-2.5.2/pom.xml#L31
This plugin declares compatibility with 1.565. It means that all default testing will be effectively running with possibly unstable configuration.
As a maintainer, I suggest bumping the core dependency to 1.609.3 as a dependency resolution measure. I would also recommend to pick the new parent pom (2.27+) with JTH 2.22 as @jglick suggests. Probably there will be no need in such hack after that.
If you have no time for it, I will take a look later
|
@oleg-nenashev I will look at it, don't worry. |
|
@oleg-nenashev done and yes, adding subversion is not necessary now. |
pom.xml
Outdated
| @@ -47,43 +47,11 @@ | |||
| </license> | |||
| </licenses> | |||
|
|
|||
| <repositories> | |||
There was a problem hiding this comment.
🐛 In such case it won't be able to retrieve Parent pom if it is missing in the local cache.
OTOH you can simplify it to
<repositories>
<repository>
<id>repo.jenkins-ci.org</id>
<url>http://repo.jenkins-ci.org/public/</url>
</repository>
</repositories>
<pluginRepositories>
<pluginRepository>
<id>repo.jenkins-ci.org</id>
<url>http://repo.jenkins-ci.org/public/</url>
</pluginRepository>
</pluginRepositories>There was a problem hiding this comment.
Well, use https but otherwise yes that is the standard blurb.
There was a problem hiding this comment.
copypasted from the wrong plugin :(
There was a problem hiding this comment.
My bad, I thought this was already included in the parent POM.
|
@oleg-nenashev I have added back the "repositories" tag and replaced |
| @@ -22,7 +21,7 @@ | |||
| @Rule | |||
| public JenkinsRule j = new JenkinsRule(); | |||
|
|
|||
| @Bug(7972) | |||
| @Issue("7972") | |||
There was a problem hiding this comment.
Please use full id, e.g. "JENKINS-7972"
|
It also fixes https://issues.jenkins-ci.org/browse/JENKINS-36654 from what I see |
oleg-nenashev
changed the title
|
Merging. Will see if I can release it today |
JENKINS-44266
@reviewbybees @oleg-nenashev @kwhetstone