[JENKINS-48463] added try-with-resources to XmlFileTest to ensure proper

file handle cleanup in the event of failure Fixed small nits regarding typo's and code cleanup Added comment explaining why Xpp3 is still included as a dep
jenkinsci · Jan 30, 2018 · 5f0f9f2 · 5f0f9f2
1 parent d994e65
commit 5f0f9f2
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 16 deletions.
diff --git a/core/pom.xml b/core/pom.xml
@@ -246,15 +246,19 @@ THE SOFTWARE.
         </exclusion>
       </exclusions>
     </dependency>
+    <!--
+    still including the xpp3 driver to ensure backwards compatibilty
+    for other plugins that may be depending on it
+    -->
     <dependency>
       <groupId>xpp3</groupId>
       <artifactId>xpp3</artifactId>
       <version>1.1.4c</version>
     </dependency>
     <dependency>
-        <groupId>net.sf.kxml</groupId>
-        <artifactId>kxml2</artifactId>
-        <version>2.3.0</version>
+      <groupId>net.sf.kxml</groupId>
+      <artifactId>kxml2</artifactId>
+      <version>2.3.0</version>
     </dependency>
     <dependency>
       <groupId>jfree</groupId>

diff --git a/core/src/main/java/hudson/util/XStream2.java b/core/src/main/java/hudson/util/XStream2.java
@@ -99,7 +99,7 @@ public class XStream2 extends XStream {
     private MapperInjectionPoint mapperInjectionPoint;
 
     /**
-     * Convinience method so we only have to change the driver in one place
+     * Convenience method so we only have to change the driver in one place
      * if we switch to something new in the future
      *
      * @return a new instance of the HierarchicalStreamDriver we want to use

diff --git a/core/src/test/java/hudson/XmlFileTest.java b/core/src/test/java/hudson/XmlFileTest.java
@@ -20,11 +20,10 @@ public class XmlFileTest {
     @Test
     public void canReadXml1_0Test() throws IOException {
         URL configUrl = getClass().getResource("/hudson/config_1_0.xml");
-        File configFile = new File(configUrl.getFile());
         XStream2  xs = new XStream2();
         xs.alias("hudson", Jenkins.class);
 
-        XmlFile xmlFile =  new XmlFile(xs, configFile);
+        XmlFile xmlFile =  new XmlFile(xs, new File(configUrl.getFile()));
         if (xmlFile.exists()) {
             Node n = (Node) xmlFile.read();
             assertThat(n.getNumExecutors(), is(2));
@@ -38,11 +37,10 @@ public void canReadXml1_0Test() throws IOException {
     @Test(expected = SAXParseException.class)
     public void xml1_0_withSpecialCharsShouldFail() throws IOException {
         URL configUrl = getClass().getResource("/hudson/config_1_0_with_special_chars.xml");
-        File configFile = new File(configUrl.getFile());
         XStream2  xs = new XStream2();
         xs.alias("hudson", Jenkins.class);
 
-        XmlFile xmlFile =  new XmlFile(xs, configFile);
+        XmlFile xmlFile =  new XmlFile(xs, new File(configUrl.getFile()));
         if (xmlFile.exists()) {
             Node n = (Node) xmlFile.read();
             assertThat(n.getNumExecutors(), is(2));
@@ -53,11 +51,10 @@ public void xml1_0_withSpecialCharsShouldFail() throws IOException {
     @Test
     public void canReadXml1_1Test() throws IOException {
         URL configUrl = getClass().getResource("/hudson/config_1_1.xml");
-        File configFile = new File(configUrl.getFile());
         XStream2  xs = new XStream2();
         xs.alias("hudson", Jenkins.class);
 
-        XmlFile xmlFile =  new XmlFile(xs, configFile);
+        XmlFile xmlFile =  new XmlFile(xs, new File(configUrl.getFile()));
         if (xmlFile.exists()) {
             Node n = (Node) xmlFile.read();
             assertThat(n.getNumExecutors(), is(2));
@@ -68,11 +65,10 @@ public void canReadXml1_1Test() throws IOException {
     @Test
     public void canReadXmlWithControlCharsTest() throws IOException {
         URL configUrl = getClass().getResource("/hudson/config_1_1_with_special_chars.xml");
-        File configFile = new File(configUrl.getFile());
         XStream2  xs = new XStream2();
         xs.alias("hudson", Jenkins.class);
 
-        XmlFile xmlFile =  new XmlFile(xs, configFile);
+        XmlFile xmlFile =  new XmlFile(xs, new File(configUrl.getFile()));
         if (xmlFile.exists()) {
             Node n = (Node) xmlFile.read();
             assertThat(n.getNumExecutors(), is(2));

diff --git a/test/src/test/java/hudson/XMLFileTest.java b/test/src/test/java/hudson/XMLFileTest.java
@@ -36,10 +36,12 @@ public void silentlyMigrateConfigsTest() throws Exception {
         // verify that we did indeed load our test config.xml
         assertThat(j.jenkins.getLabelString(), is("I am a label"));
         //verify that the persisted top level config.xml is v1.1
-        File configFile = new File(j.jenkins.getRootPath().getRemote() + File.separator + "config.xml");
+        File configFile = new File(j.jenkins.getRootDir(), "config.xml");
         assertThat(configFile.exists(), is(true));
-        BufferedReader config = new BufferedReader(new FileReader(configFile));
-        assertThat(config.readLine(), is("<?xml version='1.1' encoding='UTF-8'?>"));
-        config.close();
+
+        try (BufferedReader config = new BufferedReader(new FileReader(configFile))) {
+            assertThat(config.readLine(), is("<?xml version='1.1' encoding='UTF-8'?>"));
+            config.close();
+        }
     }
 }