Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SuiteHTMLReporter is too slow. #567

Closed
avaysberg opened this issue Nov 14, 2014 · 5 comments
Closed

SuiteHTMLReporter is too slow. #567

avaysberg opened this issue Nov 14, 2014 · 5 comments
Milestone

Comments

@avaysberg
Copy link

Hi,

I have a Problem with testng. The org.testng.reporters.SuiteHTMLReporter ist too slow. I have 271 Tests and if I turn off the reporting it needs only 7 sec. that is a output:

[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 7.288 s
[INFO] Finished at: 2014-11-14T11:13:30+01:00
[INFO] Final Memory: 10M/159M

But if I turn on the reporting, then it needs 1 m. 17 sec. and most time is used on org.testng.reporters.SuiteHTMLReporter 64655 ms:

Unit-Test2
Tests run: 271, Failures: 0, Skips: 0

[TestNG] Time taken by org.testng.reporters.JUnitReportReporter@12bcb5a9: 151 ms
[TestNG] Time taken by org.testng.reporters.XMLReporter@53c7a917: 601 ms
[TestNG] Time taken by org.testng.reporters.jq.Main@c0e1c69: 1299 ms
[TestNG] Time taken by [FailedReporter passed=0 failed=0 skipped=0]: 1 ms
[TestNG] Time taken by org.testng.reporters.SuiteHTMLReporter@52bc3811: 64655 ms
[TestNG] Time taken by org.testng.reporters.EmailableReporter2@539e922d: 104 ms
Tests run: 271, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 70.672 sec - in TestSuite
...
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:17 min
[INFO] Finished at: 2014-11-14T10:40:27+01:00
[INFO] Final Memory: 18M/222M
[INFO] ------------------------------------------------------------------------

The SuiteHTMLReporter creates always to much methods-alphabetical.html and methods.html (745 time):
...
Creating /work/test/target/surefire-reports/old/Monitoring-Core-Test/methods.html
Creating /work/test/target/surefire-reports/old/Monitoring-Core-Test/methods.html
Creating /work/test/target/surefire-reports/old/Monitoring-Core-Test/methods.html
...

With this configuration I can the problem resolved, but I think it must be better solution for creating the HMTL Reports:

<properties>
       <property>
           <name>usedefaultlisteners</name>
            <value>false</value> <!-- disabling default listeners is optional -->
         </property>
         <property>
               <name>listener</name>                                     <value>org.testng.reporters.JUnitReportReporter,
                              org.testng.reporters.jq.Main,org.testng.reporters.FailedReporter,org.testng.reporters.EmailableReporter</value>
             </property>
</properties>

And last question why the folder has name old = /surefire-reports/old/?

@psychon
Copy link
Contributor

psychon commented Oct 22, 2015

I ran into the same problem (7 seconds for the SuiteHTMLReporter) and I fixed it for me with the following patch. The problem is caused by appending many small things to files. However, this patch basically reverts 17c5221 and so I don't think this will be accepted.
So... perhaps the code should be rewritten so that it writes directly to the file? Or perhaps the file should be opened only once instead of flushing this all the time?

diff --git a/src/main/java/org/testng/reporters/SuiteHTMLReporter.java b/src/main/java/org/testng/reporters/SuiteHTMLReporter.java
index 3c75de0..c170b6b 100755
--- a/src/main/java/org/testng/reporters/SuiteHTMLReporter.java
+++ b/src/main/java/org/testng/reporters/SuiteHTMLReporter.java
@@ -354,8 +354,6 @@ public class SuiteHTMLReporter implements IReporter {
     long startDate = -1;
     sb.append("<br/><em>").append(suite.getName()).append("</em><p/>");
     sb.append("<small><i>(Hover the method name to see the test class name)</i></small><p/>\n");
-    Utils.writeFile(getOutputDirectory(xmlSuite), outputFileName, sb.toString());
-    sb = null; //not needed anymore

     Collection<IInvokedMethod> invokedMethods = suite.getAllInvokedMethods();
     if (alphabetical) {
@@ -372,13 +370,11 @@ public class SuiteHTMLReporter implements IReporter {
     }

     SimpleDateFormat format = new SimpleDateFormat("yy/MM/dd HH:mm:ss");
-    StringBuffer table = new StringBuffer();
     boolean addedHeader = false;
     for (IInvokedMethod iim : invokedMethods) {
       ITestNGMethod tm = iim.getTestMethod();
-      table.setLength(0);
       if (!addedHeader) {
-        table.append("<table border=\"1\">\n")
+        sb.append("<table border=\"1\">\n")
           .append("<tr>")
           .append("<th>Time</th>")
           .append("<th>Delta (ms)</th>")
@@ -425,7 +421,7 @@ public class SuiteHTMLReporter implements IReporter {
         startDate = iim.getDate();
       }
       String date = format.format(iim.getDate());
-      table.append("<tr bgcolor=\"" + createColor(tm) + "\">")
+      sb.append("<tr bgcolor=\"" + createColor(tm) + "\">")
         .append("  <td>").append(date).append("</td> ")
         .append("  <td>").append(iim.getDate() - startDate).append("</td> ")
         .append(td(configurationSuiteMethod))
@@ -438,9 +434,9 @@ public class SuiteHTMLReporter implements IReporter {
         .append("  <td>").append(instances).append("</td> ")
         .append("</tr>\n")
         ;
-      Utils.appendToFile(getOutputDirectory(xmlSuite), outputFileName, table.toString());
     }
-    Utils.appendToFile(getOutputDirectory(xmlSuite), outputFileName, "</table>\n");
+    sb.append("</table>\n");
+    Utils.writeFile(getOutputDirectory(xmlSuite), outputFileName, sb.toString());
   }

   /**

@juherr
Copy link
Member

juherr commented Oct 22, 2015

The commit message of 17c5221 says:

Ran a Factory test with 2000 instances before and after the changes to ensure that OOM error doesn't occur anymore.

Did you try something equivalent after your changes?

If you propose a PR with your changes and appropriate tests, I won't see any problem to accept it and I suppose @cbeust will be ok too.

@psychon
Copy link
Contributor

psychon commented Oct 22, 2015

Nope. I profiled testng with VisualVM, this pointed at writeFile and I just tested if my change improves the situation (it does). The plan was then to do a PR with this. This made me wonder why the code is the way it is and I asked git blame. Then I found that commit and so abandoned the plan to do a PR, because my patch effectively reverts that other commit and so I guess that it will reintroduce the problem.

I guess the StringBuilder has to be replaced with an OutputStreamWriter so that everything is streamed to the target file directly. However, this sounded too invasive for me and so I gave up completely (I cannot actually build testng, ant only ever complains and so I couldn't really test my changes).

@juherr
Copy link
Member

juherr commented Oct 22, 2015

Use gradle (./gradlew[.bat] test) or kobalt (./kobaltw test) if you want build and run tests.

@krmahadevan
Copy link
Member

This reporter has been replaced by a newer org.testng.reporters.jq.Main.

As part of this PR #2919 this reporter has been deprecated and will be removed off very soon.

@krmahadevan krmahadevan added this to the 7.9.0 milestone Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

4 participants