Hello Sebb, Sorry for this: - Agree with equals instead of start - For formatting as my eclipse was configured for ignoring whitespaces I didn't see all these formatting issues.
Regards Philippe On Fri, Oct 7, 2011 at 2:43 PM, sebb <[email protected]> wrote: > On 7 October 2011 13:01, sebb <[email protected]> wrote: > > On 7 October 2011 12:51, sebb <[email protected]> wrote: > >> On 7 October 2011 12:10, <[email protected]> wrote: > >>> Author: pmouawad > >>> Date: Fri Oct 7 11:10:18 2011 > >>> New Revision: 1180004 > >>> > >>> URL: http://svn.apache.org/viewvc?rev=1180004&view=rev > >>> Log: > >>> Bug 51988 - CSV Data Set Configuration does not resolve default > delimiter for header parsing when variables field is empty > >>> > >>> Modified: > >>> > jakarta/jmeter/trunk/src/components/org/apache/jmeter/config/CSVDataSet.java > >>> jakarta/jmeter/trunk/xdocs/changes.xml > >>> > >>> Modified: > jakarta/jmeter/trunk/src/components/org/apache/jmeter/config/CSVDataSet.java > >>> URL: > http://svn.apache.org/viewvc/jakarta/jmeter/trunk/src/components/org/apache/jmeter/config/CSVDataSet.java?rev=1180004&r1=1180003&r2=1180004&view=diff > >>> > ============================================================================== > >>> --- > jakarta/jmeter/trunk/src/components/org/apache/jmeter/config/CSVDataSet.java > (original) > >>> +++ > jakarta/jmeter/trunk/src/components/org/apache/jmeter/config/CSVDataSet.java > Fri Oct 7 11:10:18 2011 > >>> @@ -100,74 +100,74 @@ public class CSVDataSet extends ConfigTe > >>> public void iterationStart(LoopIterationEvent iterEvent) { > >>> FileServer server = FileServer.getFileServer(); > >>> final JMeterContext context = getThreadContext(); > >>> + String delim = getDelimiter(); > >>> + if (delim.startsWith("\\t")) { // $NON-NLS-1$ > >> > >> This is a change from the previous code, which uses .equals(). > >> I don't think startsWith() is suitable here. > >> > >>> + delim = "\t";// Make it easier to enter a Tab // > $NON-NLS-1$ > >>> + } else if (delim.length() == 0) { > >>> + log.warn("Empty delimiter converted to ','"); > >>> + delim = ","; > >>> + } > >>> if (vars == null) { > >>> String _fileName = getFilename(); > >>> String mode = getShareMode(); > >>> int modeInt = CSVDataSetBeanInfo.getShareModeAsInt(mode); > >>> - switch(modeInt){ > >>> - case CSVDataSetBeanInfo.SHARE_ALL: > >>> - alias = _fileName; > >>> - break; > >>> - case CSVDataSetBeanInfo.SHARE_GROUP: > >>> - alias = > _fileName+"@"+System.identityHashCode(context.getThreadGroup()); > >>> - break; > >>> - case CSVDataSetBeanInfo.SHARE_THREAD: > >>> - alias = > _fileName+"@"+System.identityHashCode(context.getThread()); > >>> - break; > >>> - default: > >>> - alias = _fileName+"@"+mode; // user-specified key > >>> - break; > >>> + switch (modeInt) { > >>> + case CSVDataSetBeanInfo.SHARE_ALL: > >>> + alias = _fileName; > >>> + break; > >>> + case CSVDataSetBeanInfo.SHARE_GROUP: > >>> + alias = _fileName + "@" + > System.identityHashCode(context.getThreadGroup()); > >>> + break; > >>> + case CSVDataSetBeanInfo.SHARE_THREAD: > >>> + alias = _fileName + "@" + > System.identityHashCode(context.getThread()); > >>> + break; > >>> + default: > >>> + alias = _fileName + "@" + mode; // user-specified key > >>> + break; > >>> } > >>> final String names = getVariableNames(); > >>> - if (names == null || names.length()==0) { > >>> + if (names == null || names.length() == 0) { > > > > Also, please don't change formatting when fixing bugs, unless directly > > related - e.g. adding surrounding try/catch or if statement. > > > > It makes it much harder to see what has actually changed. > > I now see that the additional changes were in the original patch. > > If a submiited patch includes unnecessary changes, we have a choice: > - either we reject it, and ask the submitter to provide a new patch > - or we apply it, but not the spurious changes. > This is reasonably easy to do in Eclipse, but can be tedious. > Apply the patch, then do a compare against the base revision. > Revert the changes that aren't relevant. > Just before commiting, recheck changes against the server version. > > Either way, commits should be the minimum necessary to solve the issue. > > >>> String header = server.reserveFile(_fileName, > getFileEncoding(), alias, true); > >>> try { > >>> - vars = CSVSaveService.csvSplitString(header, > getDelimiter().charAt(0)); > >>> + vars = CSVSaveService.csvSplitString(header, > delim.charAt(0)); > >>> firstLineIsNames = true; > >>> } catch (IOException e) { > >>> - log.warn("Could not split CSV header line",e); > >>> + log.warn("Could not split CSV header line", e); > >>> } > >>> } else { > >>> server.reserveFile(_fileName, getFileEncoding(), > alias); > >>> vars = JOrphanUtils.split(names, ","); // $NON-NLS-1$ > >>> } > >>> } > >>> - String delim = getDelimiter(); > >>> - if (delim.equals("\\t")) { // $NON-NLS-1$ > >>> - delim = "\t";// Make it easier to enter a Tab // > $NON-NLS-1$ > >>> - } else if (delim.length()==0){ > >>> - log.warn("Empty delimiter converted to ','"); > >>> - delim=","; > >>> - } > >>> - // TODO: fetch this once as per vars above? > >>> - JMeterVariables threadVars = context.getVariables(); > >>> - String line = null; > >>> + // TODO: fetch this once as per vars above? > >>> + JMeterVariables threadVars = context.getVariables(); > >>> + String line = null; > >>> + try { > >>> + line = server.readLine(alias, getRecycle(), > firstLineIsNames); > >>> + } catch (IOException e) { // treat the same as EOF > >>> + log.error(e.toString()); > >>> + } > >>> + if (line != null) {// i.e. not EOF > >>> try { > >>> - line = server.readLine(alias, getRecycle(), > firstLineIsNames); > >>> - } catch (IOException e) { // treat the same as EOF > >>> - log.error(e.toString()); > >>> - } > >>> - if (line!=null) {// i.e. not EOF > >>> - try { > >>> - String[] lineValues = getQuotedData() ? > >>> - CSVSaveService.csvSplitString(line, > delim.charAt(0)) > >>> - : JOrphanUtils.split(line, delim, false); > >>> - for (int a = 0; a < vars.length && a < > lineValues.length; a++) { > >>> - threadVars.put(vars[a], > lineValues[a]); > >>> - } > >>> - } catch (IOException e) { // Should only happen for > quoting errors > >>> - log.error("Unexpected error splitting '"+line+"' on > '"+delim.charAt(0)+"'"); > >>> - } > >>> - // TODO - report unused columns? > >>> - // TODO - provide option to set unused variables ? > >>> - } else { > >>> - if (getStopThread()) { > >>> - throw new JMeterStopThreadException("End of file > detected"); > >>> - } > >>> - for (int a = 0; a < vars.length ; a++) { > >>> - threadVars.put(vars[a], EOFVALUE); > >>> + String[] lineValues = getQuotedData() ? > >>> + CSVSaveService.csvSplitString(line, > delim.charAt(0)) > >>> + : JOrphanUtils.split(line, delim, false); > >>> + for (int a = 0; a < vars.length && a < > lineValues.length; a++) { > >>> + threadVars.put(vars[a], lineValues[a]); > >>> } > >>> + } catch (IOException e) { // Should only happen for > quoting errors > >>> + log.error("Unexpected error splitting '" + line + "' > on '" + delim.charAt(0) + "'"); > >>> + } > >>> + // TODO - report unused columns? > >>> + // TODO - provide option to set unused variables ? > >>> + } else { > >>> + if (getStopThread()) { > >>> + throw new JMeterStopThreadException("End of file > detected"); > >>> } > >>> + for (int a = 0; a < vars.length ; a++) { > >>> + threadVars.put(vars[a], EOFVALUE); > >>> + } > >>> + } > >>> } > >>> > >>> /** > >>> > >>> Modified: jakarta/jmeter/trunk/xdocs/changes.xml > >>> URL: > http://svn.apache.org/viewvc/jakarta/jmeter/trunk/xdocs/changes.xml?rev=1180004&r1=1180003&r2=1180004&view=diff > >>> > ============================================================================== > >>> --- jakarta/jmeter/trunk/xdocs/changes.xml (original) > >>> +++ jakarta/jmeter/trunk/xdocs/changes.xml Fri Oct 7 11:10:18 2011 > >>> @@ -119,6 +119,7 @@ Mirror server now uses default port 8081 > >>> <h3>General</h3> > >>> <ul> > >>> <li>Bug 51937 - JMeter does not handle missing TestPlan entry > well</li> > >>> +<li>Bug 51988 - CSV Data Set Configuration does not resolve default > delimiter for header parsing when variables field is empty</li> > >>> </ul> > >>> > >>> <!-- =================== Improvements =================== --> > >>> > >>> > >>> > >>> --------------------------------------------------------------------- > >>> To unsubscribe, e-mail: [email protected] > >>> For additional commands, e-mail: [email protected] > >>> > >>> > >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > -- Cordialement. Philippe Mouawad.
