Hi 2016-03-15 12:32 GMT+01:00 sebb <seb...@gmail.com>:
> 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) > It's usefull for you because youre are a developper. A user which see this error will not necesary understand (no knowledge of Java exception, etc.) And I don't think it's a good idea to show exception to users > > > 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) > I will try to do it asap Antonio > > > > > 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. > >> > --- > >> >
