jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: svn commit: r1201070 - in /jmeter/trunk: src/core/org/apache/jmeter/services/FileServer.java test/src/org/apache/jmeter/services/TestFileServer.java xdocs/changes.xml
Date Sat, 12 Nov 2011 11:47:28 GMT
Also if you have some time to look at:

   - 45132

Thanks

Regards

Philippe

On Sat, Nov 12, 2011 at 12:43 PM, Philippe Mouawad <
philippe.mouawad@gmail.com> 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 <sebbaz@gmail.com> wrote:
>
>> On 11 November 2011 22:09,  <pmouawad@apache.org> 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.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message