From 5f0f9f2b87be0a2d2c1dfb34df59db117f9ea90f Mon Sep 17 00:00:00 2001 From: mike cirioli Date: Tue, 30 Jan 2018 09:31:15 -0500 Subject: [PATCH] [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 --- core/pom.xml | 10 +++++++--- core/src/main/java/hudson/util/XStream2.java | 2 +- core/src/test/java/hudson/XmlFileTest.java | 12 ++++-------- test/src/test/java/hudson/XMLFileTest.java | 10 ++++++---- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 9c4faab34d..a0752203b3 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -246,15 +246,19 @@ THE SOFTWARE. + xpp3 xpp3 1.1.4c - net.sf.kxml - kxml2 - 2.3.0 + net.sf.kxml + kxml2 + 2.3.0 jfree diff --git a/core/src/main/java/hudson/util/XStream2.java b/core/src/main/java/hudson/util/XStream2.java index 13d0b359c9..4f90342ea9 100644 --- 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 index f4c3c42e02..69c85a93f7 100644 --- 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 class XmlFileTest { @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 class XmlFileTest { @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 class XmlFileTest { @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 index b384ee7dfb..1b3a9ff3ea 100644 --- a/test/src/test/java/hudson/XMLFileTest.java +++ b/test/src/test/java/hudson/XMLFileTest.java @@ -36,10 +36,12 @@ public class XMLFileTest { // 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("")); - config.close(); + + try (BufferedReader config = new BufferedReader(new FileReader(configFile))) { + assertThat(config.readLine(), is("")); + config.close(); + } } }