testng-team / testng

TestNG testing framework
https://testng.org
Apache License 2.0
1.99k stars 1.02k forks source link

SuiteHTMLReporter is too slow. #567

Closed avaysberg closed 1 year ago

avaysberg commented 10 years ago

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 commented 9 years ago

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 17c52218805e62a58f0a073b24f9b82386aa6c4a 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 commented 9 years ago

The commit message of 17c52218805e62a58f0a073b24f9b82386aa6c4a 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 commented 9 years ago

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 commented 9 years ago

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

krmahadevan commented 1 year ago

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

As part of this PR https://github.com/testng-team/testng/pull/2919 this reporter has been deprecated and will be removed off very soon.