Return-Path: X-Original-To: apmail-jakarta-dev-archive@minotaur.apache.org Delivered-To: apmail-jakarta-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id E12529404 for ; Fri, 7 Oct 2011 17:59:31 +0000 (UTC) Received: (qmail 68669 invoked by uid 500); 7 Oct 2011 17:59:31 -0000 Delivered-To: apmail-jakarta-dev-archive@jakarta.apache.org Received: (qmail 68584 invoked by uid 500); 7 Oct 2011 17:59:31 -0000 Mailing-List: contact dev-help@jakarta.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@jakarta.apache.org Delivered-To: mailing list dev@jakarta.apache.org Received: (qmail 68576 invoked by uid 99); 7 Oct 2011 17:59:31 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 07 Oct 2011 17:59:31 +0000 X-ASF-Spam-Status: No, hits=1.5 required=5.0 tests=FREEMAIL_FROM,HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS,T_TO_NO_BRKTS_FREEMAIL X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of philippe.mouawad@gmail.com designates 209.85.213.172 as permitted sender) Received: from [209.85.213.172] (HELO mail-yx0-f172.google.com) (209.85.213.172) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 07 Oct 2011 17:59:22 +0000 Received: by yxt33 with SMTP id 33so4721861yxt.31 for ; Fri, 07 Oct 2011 10:59:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=r4PN4bFZPlINmIzpbnIRz3fI33i0IEoCnSAqfPq7ELo=; b=tNUEo8UNUc64ylNaZ0nsUaVZIiDfmhAjcC++ZYaaTKsl0uXr/nXITSg2vfn78Q3Z3/ C7+E3xApEWuI/XCDjxlmplPharmnE2Z5piKiZKRrP6oM3ornyUCDAsFnwVi8gV+dYPSw J21uuu+JHmuccR2rs1cGLbnD+tRb0/AphZjgk= MIME-Version: 1.0 Received: by 10.236.129.141 with SMTP id h13mr3467002yhi.120.1318010341896; Fri, 07 Oct 2011 10:59:01 -0700 (PDT) Received: by 10.236.208.228 with HTTP; Fri, 7 Oct 2011 10:59:01 -0700 (PDT) In-Reply-To: References: <20111007111018.A7A152388A02@eris.apache.org> Date: Fri, 7 Oct 2011 19:59:01 +0200 Message-ID: Subject: Re: svn commit: r1180004 - in /jakarta/jmeter/trunk: src/components/org/apache/jmeter/config/CSVDataSet.java xdocs/changes.xml From: Philippe Mouawad To: dev@jakarta.apache.org Content-Type: multipart/alternative; boundary=20cf301af4ef3d301c04aeb93243 X-Virus-Checked: Checked by ClamAV on apache.org --20cf301af4ef3d301c04aeb93243 Content-Type: text/plain; charset=ISO-8859-1 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 wrote: > On 7 October 2011 13:01, sebb wrote: > > On 7 October 2011 12:51, sebb wrote: > >> On 7 October 2011 12:10, 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 > >>>

General

> >>>
    > >>>
  • Bug 51937 - JMeter does not handle missing TestPlan entry > well
    • > >>> +
  • Bug 51988 - CSV Data Set Configuration does not resolve default > delimiter for header parsing when variables field is empty
    • > >>>
> >>> > >>> > >>> > >>> > >>> > >>> --------------------------------------------------------------------- > >>> To unsubscribe, e-mail: notifications-unsubscribe@jakarta.apache.org > >>> For additional commands, e-mail: notifications-help@jakarta.apache.org > >>> > >>> > >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscribe@jakarta.apache.org > For additional commands, e-mail: dev-help@jakarta.apache.org > > -- Cordialement. Philippe Mouawad. --20cf301af4ef3d301c04aeb93243--