-
Notifications
You must be signed in to change notification settings - Fork 170
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-40806] tfs plugin PCT issues against 2.19.4 and 2.32.1 #144
Conversation
… parent POM, baseline version and include some necessary dependencies.
|
||
import java.io.IOException; | ||
import java.io.Serializable; | ||
|
||
public abstract class AbstractCallableCommand implements Serializable { | ||
public abstract class AbstractCallableCommand<V, T extends Throwable> extends MasterToSlaveCallable<V, T> implements Serializable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There exists also SlaveToMasterCallable, but I presume this is correct as all tests are working correctly.
<maven-surefire-plugin.version>2.19.1</maven-surefire-plugin.version> | ||
<!-- overriding old version from parent to be consistent with the above in this project --> | ||
<maven-surefire-report-plugin.version>${maven-surefire-plugin.version}</maven-surefire-report-plugin.version> | ||
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding> | ||
<maven-javadoc-plugin.version>2.8</maven-javadoc-plugin.version> | ||
<findbugs.failOnError>false</findbugs.failOnError> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Raised JENKINS-40807 to take care of them.
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. |
@olivierdagenais @davidstaheli @mosabua did you have the opportunity to review this? |
1 similar comment
I regret to inform you that I have left this project. Try @yacaovsnc |
@olivierdagenais thanks for letting me know. |
@varyvol Thanks for the pull request. If I understand this pull request correctly, it drops Java 1.6 support and requires Java 7? There is a bug fix that we'd like to push out first without change the system requirement. We will take a look at this PR soon. Thanks! |
@yacaovsnc it does because the baseline Jenkins versions requires Java 7. No problem, thanks for the heads up! |
There are functional test failures with this pull request. We need more time to properly debug and merge this pull request. Thanks for understanding. |
… JENKINS-40806
@yacaovsnc how are you executing the tests? I have merged the branch with the actual master and executed the tests with "mvn clean verify" and they seem to be working fine. |
I followed this doc. |
@yacaovsnc I see. Unfortunately I don't have a TFS to execute these functional tests against. I am suspecting that maybe the two changes in Server class or the one in RemoteChangesetVersionCommand class should be SlaveToMasterCallable and not MasterToSlaveCallable. Would it be hard for you to change those and execute the tests? |
@varyvol it's scheduled on my backlog to fix those changes. Right now I am tied up with another priority and I will take a look at this soon. |
@yacaovsnc Any news? |
@recena working on it today. |
@yacaovsnc thanks. Let me know if I can be of any help. |
@varyvol The test failure doesn't look like an issue with the Callable changes, but rather a problem with upgrading the Jenkins version. In Jenkins 1.625, How could we make search() call return with last build? |
Will fix the tests failure in another pr. Had to ignore one test as the project layout seems different and without the 'slow path' code path to find the build on disk, it's hard to change. |
@yacaovsnc thanks for investigating and working on this. |
JENKINS-40806
@yacaovsnc @davidstaheli @mosabua @reviewbybees