On 15 March 2016 at 11:15, Antonio Gomes Rodrigues <ra0...@gmail.com> wrote:
> For the moment we have only a FileNotFoundException and this error "ERROR -
> jmeter.threads.JMeterThread: Test failed!
> java.lang.IllegalArgumentException: Could not read file header line"
>
> Like you can see is not very usefull

However later in the stack trace you will see something like:

Caused by: java.io.FileNotFoundException: /workspace/JMeter/bin/xyz
(No such file or directory)

> I will work in another file problems (not present, not readable, etc.) in
> another PR
>
> For the moment I have just make this PR to alert user without stacktrace
> when he forget to setup the file name.
>
> Are you ok to have two PR or do you want I try to make only one PR with all
> the possible problem?

There is only one problem - cannot read the file.

> My opinion is to have 2 PR (this one to non setup file name and another to
> access problem to the file)

Again, I don't see the point of having two separate processes.
Just catch the IllegalArgumentException and report it (with the cause if any)

>
> Antonio
>
>
>
>
> Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
> www.avast.com
> <https://www.avast.com/fr-fr/lp-safe-emailing?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-A>
> <#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>
> 2016-03-15 11:51 GMT+01:00 sebb <seb...@gmail.com>:
>
>> On 15 March 2016 at 09:30, ra0077 <g...@git.apache.org> wrote:
>> > Github user ra0077 commented on the pull request:
>> >
>> >     https://github.com/apache/jmeter/pull/162#issuecomment-196740857
>> >
>> >     I have swap it and remove formating to be easy to read it
>> >
>> >     What I do is:
>> >     check if the user have put a file name
>> >     if not
>> >     log it to allow easy debuging
>> >     stop the test
>> >     if yes
>> >     no modification
>> >
>> >
>> >     It allow to easy debug (a clear message in log) and avoid to have an
>> >     exception to user
>> >
>> >
>> >
>> >     The diff with the trunk:
>> >
>> >     --- a/src/components/org/apache/jmeter/config/CSVDataSet.java
>> >     +++ b/src/components/org/apache/jmeter/config/CSVDataSet.java
>> >     @@ -146,6 +146,11 @@ public class CSVDataSet extends
>> ConfigTestElement
>> >
>> >          @Override
>> >          public void iterationStart(LoopIterationEvent iterEvent) {
>> >     +        String _fileName = getFilename();
>> >     +        if (_fileName.isEmpty()) {
>> >     +            log.error("No filename setup in CSV Data Set Config:
>> >     "+this.getName());
>> >     +            throw new JMeterStopThreadException("No filename setup
>> in CSV
>> >     Data Set Config: "+this.getName());
>> >     +        } else {
>> >              FileServer server = FileServer.getFileServer();
>> >              final JMeterContext context = getThreadContext();
>> >              String delim = getDelimiter();
>> >     @@ -156,7 +161,6 @@ public class CSVDataSet extends ConfigTestElement
>> >                  delim=",";
>> >              }
>> >              if (vars == null) {
>> >     -            String _fileName = getFilename();
>> >                  String mode = getShareMode();
>> >                  int modeInt =
>> CSVDataSetBeanInfo.getShareModeAsInt(mode);
>> >                  switch(modeInt){
>> >     @@ -212,6 +216,7 @@ public class CSVDataSet extends ConfigTestElement
>> >                      threadVars.put(var, EOFVALUE);
>> >                  }
>> >              }
>> >     +        }
>> >          }
>>
>> Thanks, that is more legible
>> (though the mail software causes wrapping issues)
>>
>> That looks OK.
>> Though if one wants to avoid the stacktrace appearing in the log it
>> might just be simpler to change the log message to drop the stack
>> trace for the incorrect filename case as well.
>> Does the stacktrace help for that case? I suspect not.
>> I think that would be a more useful fix.
>>
>> >     Antonio
>> >
>> >     Cet e-mail a été envoyé depuis un ordinateur protégé par Avast.
>> >     www.avast.com
>> >     <
>> https://www.avast.com/fr-fr/lp-safe-emailing?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail&utm_term=OA-2109-A
>> >
>> >     <#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>> >
>> >     2016-03-15 9:13 GMT+01:00 Vladimir Sitnikov <
>> notificati...@github.com>:
>> >
>> >     > I don't have modify these lines.
>> >     >
>> >     > @ra0077 <https://github.com/ra0077>, can you please rework the PR
>> without
>> >     > reformatting the whole method?
>> >     >
>> >     > —
>> >     > You are receiving this because you were mentioned.
>> >     > Reply to this email directly or view it on GitHub:
>> >     > https://github.com/apache/jmeter/pull/162#issuecomment-196710977
>> >     >
>> >
>> >
>> >
>> > ---
>> > If your project is set up for it, you can reply to this email and have
>> your
>> > reply appear on GitHub as well. If your project does not have this
>> feature
>> > enabled and wishes so, or if the feature is enabled but not working,
>> please
>> > contact infrastructure at infrastruct...@apache.org or file a JIRA
>> ticket
>> > with INFRA.
>> > ---
>>

Reply via email to