Also we normally put a bit more than just the commit ref on the bug report.
It's useful to include the log comment and list of files changed.
See some other recently updated bugs for examples
These details can be found from the svn ref, but it saves looking it up.
It's particularly useful when several commits are needed to complete a bug.
On 2 April 2016 at 13:15, sebb <seb...@gmail.com> wrote:
> 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.
