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

[FIXES JENKINS-15976] Correclty de-serialized settings were overwritten #833

Merged
merged 1 commit into from Jul 16, 2013

Conversation

bgloeckle
Copy link

After de-serializing changed job settings after "Save", the maven settings done by config-file-provider-plugin were overwritten blindly (presumably with fallback values). These fallback values should be set correctly in the constructor of the "Maven" class and do not need to be set explicitly after deserialization.

… "Save", the maven settings done by config-file-provider-plugin were overwritten blindly (presumably with fallback values). These fallback values should be set correctly in the constructor of the "Maven" class and do not need to be set explicitly after deserialization.
@cloudbees-pull-request-builder

core » jenkins_main_trunk #955 FAILURE
Looks like there's a problem with this pull request

@kutzi
Copy link
Member

kutzi commented Jul 14, 2013

I'm a bit struggling to understand the issue and am also a bit uncomfortable with just removing these 2 setter calls (I guess that @imod had something in mind, when adding them)
Can you create a test case for the issue?

@imod
Copy link
Member

imod commented Jul 14, 2013

as I have noted in the issue itself, I was not able to reproduce the issue...
I'll take a closer look at it again when I have some spare time.

@bgloeckle
Copy link
Author

I assume that those two method calls should be fallback values. They were added in commit b8adcb4. In fact, they were moved in that commit: From the constructor of Maven to the instantiation of the object in the Builder class.
In commit 91f0352 (a few days after the first one), the fallback calculation was added to the constructor of the Maven class again, but the calls to the setters were not removed. In fact, when calling the newInstance method, the constructor of Maven will be called, which will install the fallback values if they are needed (!= null check). If not, the constructor will use the values passed to it - and that seems to be perfectly fine. I assume that the calls to the setter methods were not removed accidentially in 91f0352.
I cannot create a test for that, although I thought of it. This is because I'd need the deserialization stuff to be going on - the best way to do this would be to write some sort of UI test that clicks the UI (Selenium?). Unfortunately, I'm not that deep into Jenkins Build Infrastructure that I could write such a test :/

@imod imod merged commit 40f3b09 into jenkinsci:master Jul 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants