On 29 February 2016 at 20:57,  <[email protected]> wrote:
> Author: pmouawad
> Date: Mon Feb 29 20:57:32 2016
> New Revision: 1732944
>
> URL: http://svn.apache.org/viewvc?rev=1732944&view=rev
> Log:
> Bug 58986 - Report/Dashboard reuses the same output directory
> Bugzilla Id: 58986
>
> Modified:
>     jmeter/trunk/bin/jmeter.properties
>     jmeter/trunk/src/core/org/apache/jmeter/JMeter.java
>     
> jmeter/trunk/src/core/org/apache/jmeter/report/dashboard/HtmlTemplateExporter.java
>     jmeter/trunk/xdocs/changes.xml
>     jmeter/trunk/xdocs/usermanual/generating-dashboard.xml
>
> Modified: jmeter/trunk/bin/jmeter.properties
> URL: 
> http://svn.apache.org/viewvc/jmeter/trunk/bin/jmeter.properties?rev=1732944&r1=1732943&r2=1732944&view=diff
> ==============================================================================
> --- jmeter/trunk/bin/jmeter.properties (original)
> +++ jmeter/trunk/bin/jmeter.properties Mon Feb 29 20:57:32 2016
> @@ -1262,6 +1262,7 @@ jmeter.reportgenerator.exporter.html.cla
>  #jmeter.reportgenerator.exporter.html.property.template_dir=report-template
>
>  # Sets the destination directory for generated html pages.
> +# This will override the value set through -o command line option
>  #jmeter.reportgenerator.exporter.html.property.output_dir=report-output
>
>  # Indicates which graph series are filtered (regular expression)
>
> Modified: jmeter/trunk/src/core/org/apache/jmeter/JMeter.java
> URL: 
> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/JMeter.java?rev=1732944&r1=1732943&r2=1732944&view=diff
> ==============================================================================
> --- jmeter/trunk/src/core/org/apache/jmeter/JMeter.java (original)
> +++ jmeter/trunk/src/core/org/apache/jmeter/JMeter.java Mon Feb 29 20:57:32 
> 2016
> @@ -110,6 +110,9 @@ public class JMeter implements JMeterPlu
>      public static final String HTTP_PROXY_USER = "http.proxyUser"; // 
> $NON-NLS-1$
>
>      public static final String JMETER_NON_GUI = "JMeter.NonGui"; // 
> $NON-NLS-1$
> +
> +    public static final String JMETER_REPORT_OUTPUT_DIR_PROPERTY =
> +            "jmeter.reportgenerator.outputdir"; //$NON-NLS-1$
>
>      // If the -t flag is to "LAST", then the last loaded file (if any) is 
> used
>      private static final String USE_LAST_JMX = "LAST";
> @@ -134,6 +137,7 @@ public class JMeter implements JMeterPlu
>      private static final int VERSION_OPT        = 'v';// $NON-NLS-1$
>      private static final int REPORT_GENERATING_OPT  = 'g';// $NON-NLS-1$
>      private static final int REPORT_AT_END_OPT      = 'e';// $NON-NLS-1$
> +    private static final int REPORT_OUTPUT_FOLDER_OPT      = 'o';// 
> $NON-NLS-1$
>
>      private static final int SYSTEM_PROPERTY    = 'D';// $NON-NLS-1$
>      private static final int JMETER_GLOBAL_PROP = 'G';// $NON-NLS-1$
> @@ -217,7 +221,10 @@ public class JMeter implements JMeterPlu
>                      "generate report dashboard only"),
>              new CLOptionDescriptor("reportatendofloadtests",
>                      CLOptionDescriptor.ARGUMENT_DISALLOWED, 
> REPORT_AT_END_OPT,
> -                    "generate report dashboard after load test")
> +                    "generate report dashboard after load test"),
> +            new CLOptionDescriptor("reportoutputfolder",
> +                    CLOptionDescriptor.ARGUMENT_REQUIRED, 
> REPORT_OUTPUT_FOLDER_OPT,
> +                    "output folder for report dashboard"),
>      };
>
>      public JMeter() {
> @@ -400,6 +407,31 @@ public class JMeter implements JMeterPlu
>                      startGui(testFile);
>                      startOptionalServers();
>                  } else {
> +                    CLOption reportOutputFolderOpt = parser
> +                            .getArgumentById(REPORT_OUTPUT_FOLDER_OPT);
> +                    if(reportOutputFolderOpt != null) {
> +                        String reportOutputFolder = 
> parser.getArgumentById(REPORT_OUTPUT_FOLDER_OPT).getArgument();
> +                        File reportOutputFolderAsFile = new 
> File(reportOutputFolder);
> +                        // We check folder does not exist or it is empty
> +                        if(!reportOutputFolderAsFile.exists() ||
> +                                // folder exists but is empty
> +                                (reportOutputFolderAsFile.isDirectory() && 
> reportOutputFolderAsFile.listFiles().length == 0)) {
> +                            if(!reportOutputFolderAsFile.exists()) {
> +                                // Report folder does not exist, we check we 
> can create it
> +                                if(!reportOutputFolderAsFile.mkdirs()) {
> +                                    throw new 
> IllegalArgumentException("Cannot create output report to:'"
> +                                            
> +reportOutputFolderAsFile.getAbsolutePath()+"' as I was not able to create 
> it");
> +                                }
> +                            }
> +                            log.info("Setting property 
> '"+JMETER_REPORT_OUTPUT_DIR_PROPERTY+"' 
> to:'"+reportOutputFolderAsFile.getAbsolutePath()+"'");
> +                            
> JMeterUtils.setProperty(JMETER_REPORT_OUTPUT_DIR_PROPERTY,
> +                                    
> reportOutputFolderAsFile.getAbsolutePath());
> +                        } else {
> +                            throw new IllegalArgumentException("Cannot 
> output report to:'"
> +                                    
> +reportOutputFolderAsFile.getAbsolutePath()+"' as it would overwrite existing 
> non empty folder");
> +                        }
> +                    }
> +

This looks really awkward.
The processing really belongs in the class/package that uses it.

It would be much cleaner just to set the property to the option value
and have the class validate it.
Otherwise the JMeter class needs to know too much about what's valid.

Also it seems wrong to validate/create the directory if it is not
going to be used.

>                      CLOption testReportOpt = parser
>                              .getArgumentById(REPORT_GENERATING_OPT);
>
>
> Modified: 
> jmeter/trunk/src/core/org/apache/jmeter/report/dashboard/HtmlTemplateExporter.java
> URL: 
> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/report/dashboard/HtmlTemplateExporter.java?rev=1732944&r1=1732943&r2=1732944&view=diff
> ==============================================================================
> --- 
> jmeter/trunk/src/core/org/apache/jmeter/report/dashboard/HtmlTemplateExporter.java
>  (original)
> +++ 
> jmeter/trunk/src/core/org/apache/jmeter/report/dashboard/HtmlTemplateExporter.java
>  Mon Feb 29 20:57:32 2016
> @@ -28,6 +28,7 @@ import java.util.regex.PatternSyntaxExce
>  import org.apache.commons.io.FileUtils;
>  import org.apache.commons.lang3.StringUtils;
>  import org.apache.commons.lang3.Validate;
> +import org.apache.jmeter.JMeter;
>  import org.apache.jmeter.report.config.ConfigurationException;
>  import org.apache.jmeter.report.config.ExporterConfiguration;
>  import org.apache.jmeter.report.config.GraphConfiguration;
> @@ -85,7 +86,8 @@ public class HtmlTemplateExporter extend
>
>      // Output directory
>      private static final String OUTPUT_DIR = "output_dir";
> -    private static final File OUTPUT_DIR_DEFAULT = new File("report-output");
> +    // Default output folder name
> +    private static final String OUTPUT_DIR_NAME_DEFAULT = "report-output";
>
>      private void addToContext(String key, Object value, DataContext context) 
> {
>          if (value instanceof String) {
> @@ -347,7 +349,7 @@ public class HtmlTemplateExporter extend
>
>          // Get output directory property value
>          File outputDir = getPropertyFromConfig(exportCfg, OUTPUT_DIR,
> -                OUTPUT_DIR_DEFAULT, File.class);
> +                getReportOutputFolder(), File.class);
>          LOG.info("Will generate dashboard in folder:" + outputDir);
>
>          // Add the flag defining whether only sample series are filtered to 
> the
> @@ -469,4 +471,16 @@ public class HtmlTemplateExporter extend
>          LOG.debug("End of template processing");
>
>      }
> +
> +    /**
> +     * @return {@link File} the folder where to output HTML report
> +     */
> +    private File getReportOutputFolder() {
> +        String globallyDefinedOutputDir = 
> JMeterUtils.getProperty(JMeter.JMETER_REPORT_OUTPUT_DIR_PROPERTY);
> +        if(!StringUtils.isEmpty(globallyDefinedOutputDir)) {
> +            return new File(globallyDefinedOutputDir);
> +        } else {
> +            return new File(JMeterUtils.getJMeterBinDir(), 
> OUTPUT_DIR_NAME_DEFAULT);
> +        }
> +    }
>  }
>
> Modified: jmeter/trunk/xdocs/changes.xml
> URL: 
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1732944&r1=1732943&r2=1732944&view=diff
> ==============================================================================
> --- jmeter/trunk/xdocs/changes.xml (original)
> +++ jmeter/trunk/xdocs/changes.xml Mon Feb 29 20:57:32 2016
> @@ -344,6 +344,7 @@ Summary
>      <li><bug>58931</bug>New Report/Dashboard : Getting font errors under 
> Firefox and Chrome (not Safari)</li>
>      <li><bug>58932</bug>Report / Dashboard: Document clearly and log what 
> report are not generated when saveservice options are not correct. Developed 
> by Florent Sabbe (f dot sabbe at ubik-ingenierie.com) and contributed by 
> Ubik-Ingenierie</li>
>      <li><bug>59055</bug>JMeter report generator : When generation is not 
> launched from jmeter/bin folder report-template is not found</li>
> +    <li><bug>58986</bug>Report/Dashboard reuses the same output 
> directory</li>
>  </ul>
>
>   <!--  =================== Thanks =================== -->
>
> Modified: jmeter/trunk/xdocs/usermanual/generating-dashboard.xml
> URL: 
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/usermanual/generating-dashboard.xml?rev=1732944&r1=1732943&r2=1732944&view=diff
> ==============================================================================
> --- jmeter/trunk/xdocs/usermanual/generating-dashboard.xml (original)
> +++ jmeter/trunk/xdocs/usermanual/generating-dashboard.xml Mon Feb 29 
> 20:57:32 2016
> @@ -444,6 +444,7 @@ jmeter.reportgenerator.apdex_statisfied_
>  jmeter.reportgenerator.apdex_tolerated_threshold=3000
>
>  # Sets the destination directory for generated html pages, it is better to 
> change it for every generation
> +# This will override the value set through -o command line option
>  # jmeter.reportgenerator.exporter.html.property.output_dir=/tmp/test-report
>
>  # Indicates which graph series are filtered (regular expression)
> @@ -481,14 +482,14 @@ jmeter.reportgenerator.exporter.html.fil
>                  <subsection name="&sect-num;.3.1 Generation from an existing 
> sample log file" anchor="report_only">
>                      <p>
>                          Use the following command :
> -                        <source>jmeter -n -g &lt;log file&gt; 
> -Jjmeter.reportgenerator.exporter.html.property.output_dir=&lt;Path to output 
> folder&gt;</source>
> +                        <source>jmeter -n -g &lt;log file&gt; -o &lt;Path to 
> output folder&gt;</source>
>                      </p>
>                  </subsection>
>
>                  <subsection name="&sect-num;.3.2 Generation after load test" 
> anchor="report_after_load_test">
>                      <p>
>                          Use the following command :
> -                        <source>jmeter -n -t &lt;test JMX file&gt; -e -l 
> &lt;test log file&gt; 
> -Jjmeter.reportgenerator.exporter.html.property.output_dir=&lt;Path to output 
> folder&gt;</source>
> +                        <source>jmeter -n -t &lt;test JMX file&gt; -e -l 
> &lt;test log file&gt; -o &lt;Path to output folder&gt;</source>
>                      </p>
>                  </subsection>
>              </subsection>
>
>

Reply via email to