On 1 March 2016 at 06:11, Philippe Mouawad <[email protected]> wrote: > On Monday, February 29, 2016, sebb <[email protected]> wrote: > >> On 29 February 2016 at 20:57, <[email protected] <javascript:;>> 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. > > for now only html exporter uses it. > But that may change > >> >> 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. > > > It just does a checking on existence and content, that's it
So why does it not do the same for the property that it overrides? Those checks are done in the reporter code, and should remain there. >> >> Also it seems wrong to validate/create the directory if it is not >> going to be used. > > > It is a matter of opinion. > > It works It does not always work properly - the option parameter is checked even if it is not going to be used, so can cause unnecessary test failures or create spurious directories > so as somebody wrote "you will have to live with it" This is new code, so changes cannot cause compatibility issues. > or feel free to code it. I will fix it. > > >> > 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="§-num;.3.1 Generation from an >> existing sample log file" anchor="report_only"> >> > <p> >> > Use the following command : >> > - <source>jmeter -n -g <log file> >> -Jjmeter.reportgenerator.exporter.html.property.output_dir=<Path to >> output folder></source> >> > + <source>jmeter -n -g <log file> -o >> <Path to output folder></source> >> > </p> >> > </subsection> >> > >> > <subsection name="§-num;.3.2 Generation after load >> test" anchor="report_after_load_test"> >> > <p> >> > Use the following command : >> > - <source>jmeter -n -t <test JMX file> -e >> -l <test log file> >> -Jjmeter.reportgenerator.exporter.html.property.output_dir=<Path to >> output folder></source> >> > + <source>jmeter -n -t <test JMX file> -e >> -l <test log file> -o <Path to output folder></source> >> > </p> >> > </subsection> >> > </subsection> >> > >> > >> > > > -- > Cordialement. > Philippe Mouawad.
