-
Bug
-
Resolution: Fixed
-
Minor
-
Jenkins >= 1.482
Original design:
- HudsonTestCase.ComputerListenerImpl and JenkinsRule.ComputerListenerImpl register channels of launched slaves in order to close at the tearDown phase.
- They are registered to Jenkins with @TestExtension
Current status:
- They does not work.
- @TestExtension can be used for classes enclosed by test case classes.
- JenkinsRule.ComputerListenerImpl does not work at all.
- HudsonTestCase.ComputerListenerImpl worked in Jenkins < 1.482 for HudsonTestCase is a super class of enclosing classes. But in Jenkins 1.482, @TestExtension works only when the encolosing class is exactly the test case class.
- This change is 5dd905f2a2 and 4557a436bf.
Problems caused by this:
- Log files of slaves are left open, and fail to delete temporary directories in Windows:
JENKINS-18259
- is related to
-
-
- Closed
-
[JENKINS-18262] @TestExtension works only when the enclosing class is exactly the test case class
ikedam
added a comment - I worked with this issue in JENKINS-18259, and sent a pull request with https://github.com/jenkinsci/jenkins/pull/814 .
ikedam
added a comment - I worked with this issue in JENKINS-18259 , and sent a pull request with https://github.com/jenkinsci/jenkins/pull/814 . HudsonTestCase.ComputerListenerImpl worked in Jenkins < 1.482 for HudsonTestCase is a super class of enclosing classes. But in Jenkins 1.482, @TestExtension works only when the encolosing class is exactly the test case class. This change is 5dd905f2a2 and 4557a436bf.
Then the obvious fix would be to correct this regression, rather than working around it. Right?
Although the Javadoc for @TestExtension says
This annotation must be used on a method/field of a test case class, or an [sic] nested type of the test case.
which would imply that the prior behavior was actually incorrect (too lenient). Not sure whether it is better to restore the prior behavior and change the specification in the Javadoc, or (as in your #814) just treat code assuming the prior undocumented behavior as in error and rewrite it.
ikedam
added a comment - Then the obvious fix would be to correct this regression, rather than working around it. Right?
Sure, it is.
And it would be also applied for JenkinsRule.
How about registering ComputerListener not with @TestExtension, but with @Extension?
ikedam
added a comment - Then the obvious fix would be to correct this regression, rather than working around it. Right?
Sure, it is.
And it would be also applied for JenkinsRule .
How about registering ComputerListener not with @TestExtension , but with @Extension ? How about registering ComputerListener not with @TestExtension, but with @Extension?
That is another possible workaround, but I would rather focus on the root issue first. Does following the suggestion in https://github.com/jenkinsci/jenkins/commit/5dd905f2a2c5316f170034c9026f6ff0eb0e2a2c#commitcomment-3437503 work?
ikedam
added a comment - I verified it works for HudsonTestCase.
Also for JenkinsRule, it must be
if (outer.isAssignableFrom(description.getTestClass()) || JenkinsRule.class.isAssignableFrom(outer)) {
ikedam
added a comment - I verified it works for HudsonTestCase .
Also for JenkinsRule , it must be
if ( outer .isAssignableFrom(description.getTestClass())
|| JenkinsRule. class. isAssignableFrom( outer )) {
Not sure if this is worth fixing. HudsonTestCase is deprecated, and JenkinsRule is not normally subclassed. Would however be expected for a JUnit 4 test which subclasses another test with a @TestExtension to load the extension. Either way, need a direct test case not related to the now-deleted computer listener.
> A. Register ComputerListenerImpl to Jenkins not using annotations(@TestExtension).
> B. Create a new annotation alternate to @TestExtension.
> C. Allow @TestExtension to accept inner classes in HudsonTestCase and JenkinsRule.
A left way:
D. Register channels not using ComputerListener.