Adapt the test for recent core #857
Conversation
| @@ -32,6 +34,16 @@ | |||
| private HttpServletResponse resp; | |||
| private FilterChain chain; | |||
|
|
|||
| @BeforeClass | |||
| public static void setProps() { | |||
| System.setProperty("hudson.security.csrf.CrumbFilter.UNPROCESSED_PATHINFO", "true"); | |||
There was a problem hiding this comment.
Isn't it safer to get the old value and restore it?
There was a problem hiding this comment.
I updated the code. Feel free to commit directly to the PR.
There was a problem hiding this comment.
When I checked out this pull request and modified pom.xml to allow it to compile with Jenkins 2.204.6, I get a null pointer exception in the unsetProps call.
java.lang.NullPointerException
at java.util.Hashtable.put(Hashtable.java:460)
at java.util.Properties.setProperty(Properties.java:166)
at java.lang.System.setProperty(System.java:798)
at hudson.plugins.git.GitStatusCrumbExclusionTest.unsetProps(GitStatusCrumbExclusionTest.java:47)
If I discard the most recent commit, then the test passes with a new spotbugs warning about a possible null pointer dereference in GitRevisionBuildParameters. That warning is a nice reminder that newer Jenkins versions include annotations which can help spotbugs find more problems for us.
I think this PR and jenkins-infra/jenkins.io#3002 highlight that it is time for the git plugin and the git client plugin to require a Jenkins version that is newer than 2.138.4.
I think that we should consider incrementing the major version of the git plugin and the git client plugin and have them depend on Jenkins 2.190.3. I think that would allow us to avoid several dependency related issues that are caused by the overly broad range of Jenkins versions supported by the git plugin and the git client plugin.
@fcojfernandez and @rsandell do you have concerns if we make git plugin require Jenkins 2.190.3 instead of 2.138.4? I'll also start a discussion in the git-plugin gitter channel
There was a problem hiding this comment.
@MarkEWaite Good catch ;) corrected
|
the code looks good and the pass in local. I haven't executed the PCT, though. I'm closing and re-opening to relaunch the CI, which seems to be a bit flaky today |
src/test/java/hudson/plugins/git/GitStatusCrumbExclusionTest.java
Outdated
Show resolved
Hide resolved
Let's give another chance before opening a INFRA ticket |
With the security release of yesterday, some tests are no longer passing for the git plugin. As it uses a mock for the crumb filter and we added some logic there, it results in NPE.
By disabling the new security correction for tests only, we prevent the test to fail on recent core version. It does not modify the behavior of the plugin that is safe, it's just the way the test is written that make it fail.
The new system property support was added in 2.204.6 but due to its nature, we can inject it in previous version without issue.