On 2 April 2016 at 13:06, Philippe Mouawad <philippe.moua...@gmail.com> wrote:
> Hi Vladimir,
> Congrats for your first Commit !
Ditto.
Please also update the changes.xml file to document the enhancement - thanks!
> Just wondering, I see this part of code has been dropped at line 290, is
> this regular ?:
> if (fileEntry.exception != null) {
> Throwable exception = fileEntry.exception;
> if (exception instanceof RuntimeException) {
> throw (RuntimeException) exception;
> }
> // Note: previous exception message should be clear enough, so
> // no additional information like "file name" needs to be added
> here
> throw new IllegalStateException(exception.getMessage(),
> exception);
> }
>
>
> Thanks
>
> On Sat, Apr 2, 2016 at 1:33 PM, <vladimirsitni...@apache.org> wrote:
>
>> Author: vladimirsitnikov
>> Date: Sat Apr 2 11:33:48 2016
>> New Revision: 1737490
>>
>> URL: http://svn.apache.org/viewvc?rev=1737490&view=rev
>> Log:
>> Bug 59153 - stop test if accessing non-existing file (e.g. empty filename
>> in CSVDataSet)
>>
>> If accessing empty or non-existing file, throw an exception so user can
>> correct the test plan.
>>
>> Bugzilla Id: 59153
>> closes #167
>>
>> Modified:
>> jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java
>> jmeter/trunk/test/src/org/apache/jmeter/config/TestCVSDataSet.java
>> jmeter/trunk/test/src/org/apache/jmeter/services/TestFileServer.java
>>
>> 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=1737490&r1=1737489&r2=1737490&view=diff
>>
>> ==============================================================================
>> --- jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java
>> (original)
>> +++ jmeter/trunk/src/core/org/apache/jmeter/services/FileServer.java Sat
>> Apr 2 11:33:48 2016
>> @@ -250,17 +250,17 @@ public class FileServer {
>> * Creates an association between a filename and a File
>> inputOutputObject,
>> * and stores it for later use - unless it is already stored.
>> *
>> - * @param filename - relative (to base) or absolute file name (must
>> not be null)
>> + * @param filename - relative (to base) or absolute file name (must
>> not be null or empty)
>> * @param charsetName - the character set encoding to use for the
>> file (may be null)
>> * @param alias - the name to be used to access the object (must not
>> be null)
>> * @param hasHeader true if the file has a header line describing the
>> contents
>> * @return the header line; may be null
>> * @throws EOFException if eof reached
>> - * @throws IllegalArgumentException if header could not be read
>> + * @throws IllegalArgumentException if header could not be read or
>> filename is null or empty
>> */
>> public synchronized String reserveFile(String filename, String
>> charsetName, String alias, boolean hasHeader) {
>> - if (filename == null){
>> - throw new IllegalArgumentException("Filename must not be
>> null");
>> + if (filename == null || filename.isEmpty()){
>> + throw new IllegalArgumentException("Filename must not be null
>> or empty");
>> }
>> if (alias == null){
>> throw new IllegalArgumentException("Alias must not be null");
>> @@ -274,20 +274,29 @@ public class FileServer {
>> log.info("Stored: "+filename+" Alias: "+alias);
>> }
>> files.put(alias, fileEntry);
>> - if (hasHeader){
>> + if (hasHeader) {
>> try {
>> - fileEntry.headerLine=readLine(alias, false);
>> - } catch (IOException e) {
>> + fileEntry.headerLine = readLine(alias, false);
>> + if (fileEntry.headerLine == null) {
>> + fileEntry.exception = new EOFException("File is
>> empty: " + fileEntry.file);
>> + }
>> + } catch (IOException | IllegalArgumentException e) {
>> fileEntry.exception = e;
>> - throw new IllegalArgumentException("Could not read
>> file header line",e);
>> - }
>> - if (fileEntry.headerLine == null) {
>> - fileEntry.exception = new EOFException("File is
>> empty: " + fileEntry.file);
>> }
>> }
>> }
>> if (hasHeader && fileEntry.headerLine == null) {
>> - throw new IllegalArgumentException("Could not read file
>> header line", fileEntry.exception);
>> + throw new IllegalArgumentException("Could not read file
>> header line for file " + filename,
>> + fileEntry.exception);
>> + }
>> + if (fileEntry.exception != null) {
>> + Throwable exception = fileEntry.exception;
>> + if (exception instanceof RuntimeException) {
>> + throw (RuntimeException) exception;
>> + }
>> + // Note: previous exception message should be clear enough, so
>> + // no additional information like "file name" needs to be
>> added here
>> + throw new IllegalStateException(exception.getMessage(),
>> exception);
>> }
>> return fileEntry.headerLine;
>> }
>> @@ -418,6 +427,9 @@ public class FileServer {
>> }
>>
>> private BufferedReader createBufferedReader(FileEntry fileEntry)
>> throws IOException {
>> + if (!fileEntry.file.canRead() || !fileEntry.file.isFile()) {
>> + throw new IllegalArgumentException("File "+
>> fileEntry.file.getName()+ " must exist and be readable");
>> + }
>> FileInputStream fis = new FileInputStream(fileEntry.file);
>> InputStreamReader isr = null;
>> // If file encoding is specified, read using that encoding,
>> otherwise use default platform encoding
>>
>> Modified:
>> jmeter/trunk/test/src/org/apache/jmeter/config/TestCVSDataSet.java
>> URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/config/TestCVSDataSet.java?rev=1737490&r1=1737489&r2=1737490&view=diff
>>
>> ==============================================================================
>> --- jmeter/trunk/test/src/org/apache/jmeter/config/TestCVSDataSet.java
>> (original)
>> +++ jmeter/trunk/test/src/org/apache/jmeter/config/TestCVSDataSet.java Sat
>> Apr 2 11:33:48 2016
>> @@ -62,10 +62,15 @@ public class TestCVSDataSet extends JMet
>> csv.setFilename("No.such.filename");
>> csv.setVariableNames("a,b,c");
>> csv.setDelimiter(",");
>> - csv.iterationStart(null);
>> - assertEquals("<EOF>",threadVars.get("a"));
>> - assertEquals("<EOF>",threadVars.get("b"));
>> - assertEquals("<EOF>",threadVars.get("c"));
>> + try {
>> + csv.iterationStart(null);
>> + fail("Bad filename in CSVDataSet -> IllegalArgumentException:
>> File No.such.filename must exist and be readable");
>> + } catch (IllegalArgumentException ignored) {
>> + assertEquals("Bad filename in CSVDataSet -> exception",
>> + "File No.such.filename must exist and be readable",
>> + ignored.getMessage());
>> + }
>> +
>>
>> csv = new CSVDataSet();
>> csv.setFilename(findTestPath("testfiles/testempty.csv"));
>>
>> 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=1737490&r1=1737489&r2=1737490&view=diff
>>
>> ==============================================================================
>> --- jmeter/trunk/test/src/org/apache/jmeter/services/TestFileServer.java
>> (original)
>> +++ jmeter/trunk/test/src/org/apache/jmeter/services/TestFileServer.java
>> Sat Apr 2 11:33:48 2016
>> @@ -129,16 +129,24 @@ public class TestFileServer extends JMet
>>
>> try {
>> FS.reserveFile(missing,charsetName,alias,true);
>> - fail("Expected IllegalArgumentException");
>> - } catch (IllegalArgumentException e) {
>> - assertTrue("Expected FNF", e.getCause() instanceof
>> java.io.FileNotFoundException);
>> + fail("Bad filename passed to FileService.reserveFile ->
>> IllegalArgumentException: Could not read file header line for file
>> no-such-file");
>> + } catch (IllegalArgumentException ignored) {
>> + assertEquals("Bad filename passed to FileService.reserveFile
>> -> exception",
>> + "Could not read file header line for file
>> no-such-file",
>> + ignored.getMessage());
>> + assertEquals("Bad filename passed to FileService.reserveFile
>> -> exception",
>> + "File no-such-file must exist and be readable",
>> ignored.getCause().getMessage());
>> }
>> // Ensure second invocation gets same behaviour
>> try {
>> FS.reserveFile(missing,charsetName,alias,true);
>> - fail("Expected IllegalArgumentException");
>> - } catch (IllegalArgumentException e) {
>> - assertTrue("Expected FNF", e.getCause() instanceof
>> java.io.FileNotFoundException);
>> + fail("Bad filename passed to FileService.reserveFile ->
>> IllegalArgumentException: Could not read file header line for file
>> no-such-file");
>> + } catch (IllegalArgumentException ignored) {
>> + assertEquals("Bad filename passed to FileService.reserveFile
>> -> exception",
>> + "Could not read file header line for file
>> no-such-file",
>> + ignored.getMessage());
>> + assertEquals("Bad filename passed to FileService.reserveFile
>> -> exception",
>> + "File no-such-file must exist and be readable",
>> ignored.getCause().getMessage());
>> }
>> }
>>
>>
>>
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.
