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

Allow option showRawYaml in Jenkinsfiles #519

Merged
merged 7 commits into from
Jul 3, 2019

Conversation

OliverNocon
Copy link
Contributor

Add option to hide pod yaml also in Jenkinsfile pipeline execution by setting showRawYaml: false.

https://issues.jenkins-ci.org/browse/JENKINS-57717

@OliverNocon
Copy link
Contributor Author

Change has been successfully tested e2e in Jenkins instance running in a Kubernetes cluster.

@Vlatombe
Copy link
Member

Thanks for this. Would you mind adding a small unit test ?

@OliverNocon
Copy link
Contributor Author

sure, could you please point me to the place where I can find the test for the non-pipeline use-case. I will then take this over.

@OliverNocon
Copy link
Contributor Author

Thanks for the pointer. I added a simple Unit test now.
Involving ContainerTemplate does not look feasible to me here.

I looked into testing the additions to class PodTemplateStepExecution.
Unfortunately, a more detailed but simple unit test for this class does not look as easy to me without refactoring the class itself since the constructor does not set the properties but rather the start() method which already requires Jenkins.getInstance().
I don't want to go into refactoring here though.

Hope this is fine ...

@Vlatombe
Copy link
Member

Vlatombe commented Jul 2, 2019

This unit test is not really useful (merely testing the getter). Usually anything involving something related to pipeline steps is better tested using an integration test.

Typically in your case, something that would look like

using the showRawYaml:true in the pod template definition, then asserting that the yaml definition is not printed in the build logs

@OliverNocon
Copy link
Contributor Author

I fully agree that testing getters and setters is not of much value.
I would have wished for a better unit test capability of PodTemplateStepExecution in a way that I could just run the start() method on PodTemplateStep and then assert that the correct value made it into newTemplate.
That could make testing faster in my opinion and easier for new contributors ...

Anyway, thank you for your hint.
Since I cannot run the tests locally I gave it a try with an integration test and a best guess now ...

@Vlatombe Vlatombe merged commit ba259c8 into jenkinsci:master Jul 3, 2019
@OliverNocon
Copy link
Contributor Author

@Vlatombe thanks for helping out here!

@OliverNocon OliverNocon deleted the podYaml branch July 3, 2019 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants