Also if you have some time to look at: - 45132
Thanks Regards Philippe On Sat, Nov 12, 2011 at 12:43 PM, Philippe Mouawad < [email protected]> wrote: > Hello Sebb, > I rolled back and changed approach, you were right I missed some major > differences in the 3 methods. > > In my new commits, I had to change a catch Exception in one test because I > made the 3 Fileserver method use the same exception IllegalStateException > for the same error. > I impacted client methods to use this new exception. > > I stop work on this for now, not sure we can do more without regressions, > I don't think the 3 methods do the same job. > > - setBasedir and setBase differ in the fact setBasedir accepts null > parameter (strange for me but it's like this) > - Regarding setBase, I wonder if it is OK to use setBaseForScript and > passe a new File(jmxBase, scriptName)? what if client and remote are not in > the same folder ? > > > If you have some time can you look at: > > - 47850 <https://issues.apache.org/bugzilla/show_bug.cgi?id=47850> > - And my last note in > 43294<https://issues.apache.org/bugzilla/show_bug.cgi?id=43294> > > > Thanks > Regards > Philippe > > On Fri, Nov 11, 2011 at 11:35 PM, sebb <[email protected]> wrote: > >> On 11 November 2011 22:09, <[email protected]> wrote: >> > Author: pmouawad >> > Date: Fri Nov 11 22:09:06 2011 >> > New Revision: 1201070 >> > >> > URL: http://svn.apache.org/viewvc?rev=1201070&view=rev >> > Log: >> > Bug 52150 - FileServer has 3 confusingly similar methods to set the >> file base >> >> -1 >> >> Although the result looks tidier, the behaviour has changed; please >> revert. >> >> > Modified: >> > jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java >> > jmeter/trunk/test/src/org/apache/jmeter/services/TestFileServer.java >> >> By all means add new tests, but if existing tests have to be changed, >> this is a sign that the update caused the code behaviour to change. >> >> > jmeter/trunk/xdocs/changes.xml >> > >> > Modified: >> jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java?rev=1201070&r1=1201069&r2=1201070&view=diff >> > >> ============================================================================== >> > --- jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java >> (original) >> > +++ jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java >> Fri Nov 11 22:09:06 2011 >> > @@ -113,16 +113,12 @@ public class FileServer { >> > * @throws IOException if there is a problem resolving the file name >> > */ >> > public synchronized void setBasedir(String basedir) throws >> IOException { >> > - if (filesOpen()) { >> > - throw new IOException("Files are still open, cannot change >> base directory"); >> > - } >> > - files.clear(); >> > if (basedir != null) { >> > - base = new File(basedir); >> > + File base = new File(basedir); >> > if (!base.isDirectory()) { >> > base = base.getParentFile(); >> > } >> > - log.info("Set new base='"+base+"'"); >> > + setBase(base); >> > } >> > } >> > >> > @@ -139,14 +135,9 @@ public class FileServer { >> > if (scriptPath == null){ >> > throw new IllegalArgumentException("scriptPath must not be >> null"); >> > } >> > - if (filesOpen()) { >> > - throw new IllegalStateException("Files are still open, >> cannot change base directory"); >> > - } >> > - files.clear(); >> > - // getParentFile() may not work on relative paths >> > + File parentFolder = >> scriptPath.getAbsoluteFile().getParentFile(); >> > + setBase(parentFolder); >> > setScriptName(scriptPath.getName()); >> > - base = scriptPath.getAbsoluteFile().getParentFile(); >> > - log.info("Set new base '"+base+"'"); >> > } >> > >> > /** >> > >> > Modified: >> jmeter/trunk/test/src/org/apache/jmeter/services/TestFileServer.java >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/services/TestFileServer.java?rev=1201070&r1=1201069&r2=1201070&view=diff >> > >> ============================================================================== >> > --- >> jmeter/trunk/test/src/org/apache/jmeter/services/TestFileServer.java >> (original) >> > +++ >> jmeter/trunk/test/src/org/apache/jmeter/services/TestFileServer.java Fri >> Nov 11 22:09:06 2011 >> > @@ -94,13 +94,21 @@ public class TestFileServer extends JMet >> > assertFalse("Should not have any files open",FS.filesOpen()); >> > assertEquals("a1,b1,c1,d1",FS.readLine(infile)); >> > try { >> > - FS.setBasedir("x"); >> > - fail("Expected IOException"); >> > - } catch (IOException ignored){ >> > + FS.setBasedir(System.getProperty("java.io.tmpdir")); >> > + fail("Expected IllegalStateException"); >> > + } catch (IllegalStateException ignored){ >> > + assertEquals("Files are still open, cannot change base >> directory", ignored.getMessage()); >> > } >> > FS.closeFile(infile); >> > - FS.setBasedir("y"); >> > + FS.setBasedir(System.getProperty("java.io.temp")); >> > FS.closeFiles(); >> > + >> > + try { >> > + FS.setBasedir("x"); >> > + fail("Expected IllegalArgumentException"); >> > + } catch (IllegalArgumentException ignored){ >> > + assertEquals("jmxBase must not be null", >> ignored.getMessage()); >> > + } >> > } >> > >> > public void testRelative() throws Exception { >> > >> > Modified: jmeter/trunk/xdocs/changes.xml >> > URL: >> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1201070&r1=1201069&r2=1201070&view=diff >> > >> ============================================================================== >> > --- jmeter/trunk/xdocs/changes.xml (original) >> > +++ jmeter/trunk/xdocs/changes.xml Fri Nov 11 22:09:06 2011 >> > @@ -208,6 +208,7 @@ these occurs, Sampler is marked as faile >> > <li>Bug 52116 - Allow to add (paste) entries from the clipboard to an >> arguments list</li> >> > <li>Bug 51091 - New function returning the name of the current "Test >> Plan"</li> >> > <li>Bug 52160 - Don't display TestBeanGui items which are flagged as >> hidden</li> >> > +<li>Bug 52150 - FileServer has 3 confusingly similar methods to set >> the file base</li> >> > </ul> >> > >> > <h2>Non-functional changes</h2> >> > >> > >> > >> > > > > -- > Cordialement. > Philippe Mouawad. > > > > -- Cordialement. Philippe Mouawad.
