Bump parent pom to 3.54 #99
Conversation
slonopotamus
force-pushed
the
parent-pom-3.54
branch
from
b7ac35c
to
1cc1ee7
Compare
This test reproduces the problem that happens in jenkinsci/lockable-resources-plugin#170
| public JenkinsRule j = new JenkinsRule(); | ||
|
|
||
| @Test | ||
| public void hardKill() throws Exception { |
There was a problem hiding this comment.
What is the problem that this test is reproducing? Did it hang forever with older implementations of WorkflowRun.doKill or something?
There was a problem hiding this comment.
Please, look at jenkinsci/lockable-resources-plugin#170.
They have issues with upgrading to modern JTH, LockStepHardKillTest leaves open file descriptors after test ends and this is detected by jenkinsci/jenkins-test-harness#166.
However, this failure isn't related to lockable-resources-plugin at all, it is local to workflow-support-plugin + workflow-api. That's why I am creating a test here that is a stripped-down version of LockStepHardKillTest demonstrating the same issue.
Note that issue cannot be detected by JMH without upgrading parent POM, that's why I put these changes together.
You may see how this test fails without workflow-api upgrade in build 3 of current PR.
Overall, I'm currently walking through different Jenkins plugins and updating parent pom to 3.54 (you've seen some of my PRs already). Some plugins update cleanly, some encounter different issues related to jenkinsci/jenkins-test-harness#166.
There was a problem hiding this comment.
Ok, so if I understand correctly, the problem is that older versions of workflow-api leave the log file opened in some cases (I guess related to the bugs fixed in versions 2.31, 2.32, and 2.33 of that plugin), which causes test failures on Windows because the temp directories for the tests can't be deleted? I'm just confused why workflow-support has anything to do with it, since you didn't update any code related to SemaphoreStep here, I would think you could see the same problem with waitUntil or maybe even sleep.
You should be able to update the version of workflow-api that lockable-resources depends on (right now it picks up an old version through the workflow-support dependency). From a quick look at the APIs it is using, I think that it would make more sense for lockable-resources to only depend on workflow-step-api and workflow-api in compile scope, and move the workflow-support non-tests-classifier dependency to test scope.
There was a problem hiding this comment.
(Adding the test here is fine, thanks for adding it! I just don't think it matters for lockable-resources)
There was a problem hiding this comment.
the problem is that older versions of workflow-api leave the log file opened in some cases
Right. Newer parent pom brings in newer JTH. Newer JTH is smart enough to detect a bug in older workflow-api versions. This bug can be demonstrated by a (useful, IMO) test that terminates build during semaphore step. lockable-resources have such test. workflow-support-plugin doesn't (until this PR). Fixing it requires updating dependency on workflow-api. And that's what I am doing here.
I do not say that this PR is a prerequisite to fix lockable-resources, they have other options to go.
I would think you could see the same problem with waitUntil or maybe even sleep.
Yes, though semaphore is more deterministic (no hardcoded timeouts and stuff like that).
|
Note that there are currently issues with ci.jenkins.io, so if master build fails after merging this, just try rerunning it later when infra issues are resolved. |
No description provided.