-
Notifications
You must be signed in to change notification settings - Fork 127
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-26143] Tests #72
Conversation
Failing test looks unrelated, probably a slightly outdated core that doesn't have jenkinsci/jenkins#3342 yet. |
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.
Looks good, but we won’t merge this for a while due to the core bump.
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.
Can be simplified but good enough.
@@ -64,7 +64,8 @@ THE SOFTWARE. | |||
</pluginRepositories> | |||
<properties> | |||
<java.level>8</java.level> | |||
<jenkins.version>2.62</jenkins.version> | |||
<jenkins-core.version>2.111-20180312.014148-2</jenkins-core.version> | |||
<jenkins-war.version>2.111-20180312.014313-2</jenkins-war.version> |
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.
You should also delete the now-older baseline from Jenkinsfile
, thus simply:
buildPlugin()
@@ -99,6 +101,7 @@ | |||
ParametersDefinitionProperty.DescriptorImpl.class.isAnnotationPresent(Symbol.class) && // "parameters" | |||
BooleanParameterDefinition.DescriptorImpl.class.isAnnotationPresent(Symbol.class) && // "booleanParam" | |||
StringParameterDefinition.DescriptorImpl.class.isAnnotationPresent(Symbol.class) && // "string" | |||
ChoiceParameterDefinition.DescriptorImpl.class.isAnnotationPresent(Symbol.class) && // "choice" and "choiceParam" |
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.
Well clearly if we have this new core baseline, HAVE_SYMBOL == true
, so we can just delete all these switches.
@Test | ||
public void testChoiceParameterSnippetizer() throws Exception { | ||
//new SnippetizerTester(r).assertGenerateSnippet(); | ||
new SnippetizerTester(r).assertRoundTrip(new JobPropertyStep(Arrays.asList(new ParametersDefinitionProperty(new ChoiceParameterDefinition[] { |
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.
use varargs
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.
#81 subsumes this.
Reminder that that does not mean this needs to be closed. Merging #81 would automatically mark this as merged as well. |
Companion PR to core change jenkinsci/jenkins#3014
Built against jenkinsci/jenkins@7b8e4b1