htmlreporter: appendChild of a specDone only all 500ms

[HolgerJeromin]

Description

appendChild with all success full spec costs time because it forces the browser to rerender every dot.

Motivation and Context

Ran 2 of 4822 specs => 27 seconds with the main branch
Screenshot from devtools performance trace shows constant "layout", "update layer tree", "paint":

Ran 2 of 4822 specs => 12 seconds with this branch
Screenshot from devtools performance trace shows "layout", "update layer tree", "paint" only every 500ms:

How Has This Been Tested?

Ran many tests with success and failures.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[sgravrock]

I appreciate the contribution, but this looks pretty broken in its current state. The tests fail in browsers, the postttest script fails on node, and the reporter crashes on startup because it tries to access document.body before it's defined.

Please read the contributing guide and pay special attention to the parts about testing. You should be able to reproduce the crash by running npm run build, running npm run serve, and viewing the tests in a browser. In this case I'd especially want to see tests that use the mock clock to exercise both the case where there isn't already a timeout queued up and the case where there is.

It would also help to know more a little more about the problem. What browser did you get those numbers from? Are your tests mostly synchronous our asynchronous?

If you write tests for the new functionality, get it working, and fix the existing test failures, I'd be happy to take a look. Fair warning: it might take a while. This is an area where historically one person's optimization has been another person's performance regression, so I'll have to do a fair bit of manual testing.

[HolgerJeromin]

I adjusted the code a bit, but still need to adjust the tests.

It would also help to know more a little more about the problem. What browser did you get those numbers from?

The screenshot is from a stable Chrome v 89.0.4389.114
But we see laggy tests (because of slow rendering of the dots) since years.

Are your tests mostly synchronous our asynchronous?

Most tests are sync ones.

[sgravrock]

That sounds like a pretty similar workload to Jasmine's own test suite, so I'm surprised that you're seeing such worse performance than I am. If you run Jasmine's test suite in the same browser and click on a single test (so that HTML rendering should dominate the run time), how long does it take?

[HolgerJeromin]

That sounds like a pretty similar workload to Jasmine's own test suite, so I'm surprised that you're seeing such worse performance than I am. If you run Jasmine's test suite in the same browser and click on a single test (so that HTML rendering should dominate the run time), how long does it take?

Even with jasmine own tests suite
2 specs, 0 failures, randomized with seed 21487
http://localhost:8888/?spec=matchersUtil%20equals%20Building%20diffs%20for%20asymmetric%20equality%20testers
my change has a better timing:

faster-display branch (1.8 sec for the painting block)

main branch (3.2 sec for the painting block)

Probably my own tests could be optimized for not running 😀 but I take every improvement.

[HolgerJeromin]

Fixed the current unit tests and added test to the delay.

[sgravrock]

Sorry, but there are still some problems with this.

1. npm test still fails:

[...]/src/html/HtmlReporter.js
  133:29  error  Missing space before opening brace  space-before-blocks

[...]/spec/html/HtmlReporterSpec.js
  113:18  error  Parsing error: Unexpected token )

✖ 2 problems (2 errors, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

2. The tests fail to load with a syntax error on Internet Explorer.
3. Ironically enough, your changes nearly quadruple the run time of Jasmine's test suite.

The IE syntax error and the second lint error are both because you're using Javascript features (particularly arrow functions and promises) that don't exist on all of the browsers that Jasmine runs on. Jasmine code needs to be written in straight ES5 with no newer language or library features, except in certain specific situations.

To speed up the tests, please use the mock clock instead of setTimeout in the tests.

I'd also like to see the delay dropped from 500ms to something smaller, like 100ms or less. 500ms is visibly jerky, and I can't measure a noticeable difference between 500ms and 100ms over 5000 dots in any browser except Safari.

[HolgerJeromin]

Thanks again for your feedback.

Sorry for ignoring jasmine.clock till now. This was really easy.
Reduced timeout to 100ms (an improvement in my own tests, too).
Removed arrow functions and tested in ie11.

[sgravrock]

This is getting closer but it's not quite there yet. The tick calls in the tests should be for 100ms, not 1000, since that's how long the corresponding setTimeout calls are for. And the formatting check at the end of npm test fails:

Checking formatting...
src/html/HtmlReporter.js
spec/html/HtmlReporterSpec.js
Code style issues found in the above file(s). Forgot to run Prettier?

You can fix the formatting problems by running npm run cleanup. Please don't push again unless npm test succeeds. All of your commits so far have failed that check. If you're having trouble understanding the output, you can check the exit code. 0 means success. Anything other than 0 means there are still problems.

[sgravrock]

Closing due to inactivity, and because I can no longer reproduce any performance improvement with this patch. On the hardware & browsers that I currently have access to, debouncing makes things less smooth but no faster. I suspect that whatever improvements were to be had here have been overtaken by improvements in browsers.