Hi Vladimir,
Congrats for your first Commit !
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.
