JENKINS-55920: add CasC support to GitHubPluginConfig w/Tests #210
Conversation
er1c
force-pushed
the
HOTFIX-add-casc-support
branch
from
bdfd613
to
6a56228
Compare
pom.xml
Outdated
| <dependency> | ||
| <groupId>io.jenkins</groupId> | ||
| <artifactId>configuration-as-code</artifactId> | ||
| <version>1.0</version> |
There was a problem hiding this comment.
use the latest released version please (1.7) https://github.com/jenkinsci/configuration-as-code-plugin/releases
|
@er1c would be great if you could rebase: |
er1c
force-pushed
the
HOTFIX-add-casc-support
branch
from
6a56228
to
ea585ca
Compare
There was a problem hiding this comment.
Please see the updated https://github.com/jenkinsci/configuration-as-code-plugin/blob/master/docs/PLUGINS.md#how-to-test
I suggest you add an export test as well 😄
src/test/java/org/jenkinsci/plugins/github/config/ConfigAsCodeTest.java
Outdated
Show resolved
Hide resolved
er1c
force-pushed
the
HOTFIX-add-casc-support
branch
from
ea585ca
to
4c98e97
Compare
|
This has been rebased, and the outstanding review comments addressed. |
src/main/resources/org/jenkinsci/plugins/github/config/GitHubPluginConfig/config.groovy
Show resolved
Hide resolved
| @Test | ||
| @ConfiguredWithCode("configuration-as-code.yml") | ||
| public void export_configuration() throws Exception { | ||
| /* TODO (From JCASC): Need to provide some YAML assertion library so that the resulting exported yaml |
There was a problem hiding this comment.
We need to update the plugin docs but it is being tested in this way in a few plugins and in the configuration-as-code repo:
https://github.com/jenkinsci/azure-keyvault-plugin/blob/master/src/test/java/org/jenkinsci/plugins/azurekeyvaultplugin/ConfigAsCodeTest.java#L33
There was a problem hiding this comment.
I've updated the docs now:
https://github.com/jenkinsci/configuration-as-code-plugin/blob/master/docs/PLUGINS.md#model-export-test
| @@ -0,0 +1,17 @@ | |||
| unclassified: | |||
|
|
|||
| githubpluginconfig: | |||
There was a problem hiding this comment.
maybe another more minimal test which shows what you need if you set manage hooks false and no hook url?
|
@er1c Thanks for the good work! Thanks |
| public void export_configuration() throws Exception { | ||
| /* TODO (From JCASC): Need to provide some YAML assertion library so that the resulting exported yaml | ||
| stream can be checked for expected content. */ | ||
| ConfigurationAsCode.get().export(System.out); |
There was a problem hiding this comment.
never user System.out, use loggers if you want to print something.
|
Hi, we're having this issue where |
|
I'd guess https://github.com/jenkinsci/github-plugin/pull/210/files#r277181851 is still a blocker, no? |
|
Any movement on this? My team is blocked by not being able to set the |
|
Basically the only thing to set manually at startup or after a Jenkins upgrade. I would be happy to help with this issue, I can do some testing but I am no Java/Groovy dev. |
|
Based on @er1c's Github contribution history, it looks like he probably doesn't have bandwidth to work on this anymore. Does anyone here have the Java experience to take over this PR? I am also hitting this particular issue. |
I'm currently slammed and don't have time for this. For anyone who is blocked, you can work around it via: https://gist.github.com/er1c/c057640d43c0dce149b0a0c23d1ce153#file-jenkins-github-yaml-L80 I think the remaining "todos" are straightforward, it mostly just needs someone to steward the PR. You probably don't need much java experience :) |
… simply deprecate the overrideHookUrl, and adding additional tests
er1c
force-pushed
the
HOTFIX-add-casc-support
branch
from
4c98e97
to
b08bc56
Compare
|
Thanks @er1c for picking this up 👍 |
This is based upon both #205 and #209
I can rebase the PR once #209 is addressed to make the PR cleaner.
This change is