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.

Sign up for GitHub

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

Jump to bottom

[ISSUE JENKINS-25280] TestObject has no getFullDisplayName #3

Merged
merged 1 commit into from
Oct 25, 2014
Merged

[ISSUE JENKINS-25280] TestObject has no getFullDisplayName #3

merged 1 commit into from
Oct 25, 2014

Conversation

jtnord
Copy link
Member

@jtnord jtnord commented Oct 23, 2014

use displayName not fullDisplayName which does not exist on generic TestObjects.

@jtnord
[ISSUE JENKINS-25280] TestObject has no getFullDisplayName
f371349 
use displayName not fullDisplayName which does not exist.
@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@jglick jglick self-assigned this Oct 25, 2014
@jglick
Copy link
Member

jglick commented Oct 25, 2014

Does not actually pass tests, but I am working on an alternate fix.

jglick referenced this pull request in jenkinsci/jenkins Oct 25, 2014
@jtnord
[FIXED JENKINS-25280] Failed tests do not get displayed properly in t…
e135651 
…he overview

there is no such mothod getFullDisplayName in a TestObject - only getDisplayName

Change-Id: Idd454be00a22ee3c93f661a3ee3ae322a25c271d
@jglick
Copy link
Member

jglick commented Oct 25, 2014

Cf. jenkinsci/jenkins@e135651

@jglick jglick merged commit f371349 into master Oct 25, 2014
jglick added a commit that referenced this pull request Oct 25, 2014
@jglick
[JENKINS-25280] Variant fix of #3 that passes tests: just add getFull…
28bb359 
…DisplayName().
@jglick jglick deleted the JENKINS-25280 branch October 25, 2014 22:39
@KostyaSha
Copy link
Member

So now there is:
getName() -> Gets the name of this object.
getFullName() @SInCE 1.551 -> Gets the full name of this object.
getSafeName() -> Gets the version of {@link #getName()} that's URL-safe
getDescription() -> no javadoc in interface
and
getFullDisplayName() -> no javadoc description
and
getDisplayName() ?

@jtnord
Copy link
Member Author

jtnord commented Oct 26, 2014

The test failure seemed unrelated and was failing in the previous change.

@jglick
Copy link
Member

jglick commented Oct 27, 2014

getFullDisplayName was already defined on subtypes and implicitly assumed to be in the supertype by its call from Jelly. Adding Javadoc would be nice.

The test failure was reproducible for me until I adjusted the fix.

@KostyaSha
Copy link
Member

Why not fix jelly and use methods defined in interface/abstract?

@KostyaSha
Copy link
Member

@daniel-beck found possible example https://github.com/jenkinsci/jenkins/blob/master/core/src/main/resources/hudson/diagnosis/OldDataMonitor/manage.jelly#L87
@jglick can this be used? Can we not break backward compatibility for plugins jenkinsci/cucumber-testresult-plugin@cb4da12 ?

@jglick
Copy link
Member

jglick commented Oct 27, 2014

Why not fix jelly and use methods defined in interface/abstract?

Because the relevant method was not defined in a sufficiently abstract class, only on certain subclasses specific to JUnit.

can this be used?

Yes I considered that idiom, reliant on dynamic/duck typing, but preferred the simpler and more comprehensible fix which would work even if Jelly scripts were rewritten to some statically-typed language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jglick jglick

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jtnord @jenkinsadmin @jglick @KostyaSha