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-40806] tfs plugin PCT issues against 2.19.4 and 2.32.1 #144

Merged
merged 2 commits into from
May 11, 2017

Conversation

varyvol
Copy link

@varyvol varyvol commented Jan 4, 2017

JENKINS-40806

  • Increase parent POM version.
  • Increase baseline.
  • Include necessary dependencies.
  • Include "escape-by-default" property in Jelly files.
  • Use MasterToSlaveCallable needed in new baseline version.

@yacaovsnc @davidstaheli @mosabua @reviewbybees

… 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 {
Copy link
Author

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>
Copy link
Author

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.

@ghost
Copy link

ghost commented Jan 11, 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.

@varyvol
Copy link
Author

varyvol commented Jan 23, 2017

@olivierdagenais @davidstaheli @mosabua did you have the opportunity to review this?

@varyvol
Copy link
Author

varyvol commented Feb 6, 2017

1 similar comment
@varyvol
Copy link
Author

varyvol commented Mar 8, 2017

@olivierdagenais
Copy link
Member

I regret to inform you that I have left this project. Try @yacaovsnc

@varyvol
Copy link
Author

varyvol commented Mar 8, 2017

@olivierdagenais thanks for letting me know.

@yacaovsnc
Copy link
Contributor

@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!

@varyvol
Copy link
Author

varyvol commented Mar 14, 2017

@yacaovsnc it does because the baseline Jenkins versions requires Java 7.

No problem, thanks for the heads up!

@yacaovsnc
Copy link
Contributor

Failed tests: 
  FunctionalTest.createLabel:224 expected:<NONE> but was:<INCOMPARABLE>
  FunctionalTest.oldPollingFallback:616 expected:<hudson.scm.PollingResult@434c179e> but was:<hudson.scm.PollingResult@46199519>
  FunctionalTest.upgradeEncodedPassword:666 expected:<NONE> but was:<INCOMPARABLE>
  FunctionalTest.useWebProxyServer:729 expected:<NONE> but was:<INCOMPARABLE>

Tests run: 12, Failures: 4, Errors: 0, Skipped: 1

There are functional test failures with this pull request. We need more time to properly debug and merge this pull request. Thanks for understanding.

@varyvol
Copy link
Author

varyvol commented Apr 3, 2017

@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.

@yacaovsnc
Copy link
Contributor

I followed this doc.

@varyvol
Copy link
Author

varyvol commented Apr 3, 2017

@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?

@yacaovsnc
Copy link
Contributor

@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.

@recena
Copy link

recena commented May 5, 2017

@yacaovsnc Any news?

@yacaovsnc
Copy link
Contributor

@recena working on it today.

@varyvol
Copy link
Author

varyvol commented May 10, 2017

@yacaovsnc thanks. Let me know if I can be of any help.

@yacaovsnc
Copy link
Contributor

@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, AbstractLazyLoadRunMap.search(final int n, final Direction d) seem are looking for exact matches; while in v1.58, it goes through a "slow path" to find the build from disk. I verified the workspaces on disk, it seems in both versions the builds were fine. Just in AbstractProject.java:1309, when we call "getLastBuild()", we are always getting null.

How could we make search() call return with last build?

@yacaovsnc
Copy link
Contributor

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.

@varyvol
Copy link
Author

varyvol commented May 15, 2017

@yacaovsnc thanks for investigating and working on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants