fix(@wdio/reporter): transform scripts in TestStats (#13209)

[johnp]

Proposed changes

Fixes #13209 and dequelabs/axe-core-npm#1086. Simply adds a Map which points any script string larger than 1KB at the same in-memory script instance. Now, @axe/webdriverio only leaks the script a total of three times (due whitespace differences in @axe/webdriverio), regardless of the number of Axe calls. The cutoff point at 1KB is arbitrary. I'm pretty sure we could set it higher to avoid trashing the Map too much, while still benefiting from deduplication, but I don't have any knowledge as to how big execute scripts in the ecosystem usually are, especially one's which are executed so many times, that their additive size could become problematic.

Changed to instead transform the scripts strings into short representations w/ byte count and, if easily available, the first function name (can make it easier to locate than the byte count alone).

Types of changes

• Polish (an improvement for an existing feature)
• Bugfix (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 not work as expected)
• Documentation update (improvements to the project's docs)
• Specification changes (updates to WebDriver command specifications)
• Internal updates (everything related to internal scripts, governance documentation and CI files)

Checklist

• I have read the CONTRIBUTING doc
• I have added tests that prove my fix is effective or that my feature works
• I have added the necessary documentation (if appropriate)
• I have added proper type definitions for new commands (if appropriate)

Backport Request

//: # (The current main branch is the development branch for WebdriverIO v9. If your change should be released to the current major version of WebdriverIO (v8), please raise another PR with the same changes against the v8 branch.)

I'd like to at least backport the fix to v8, if not the test as well.

• Back-ported PR at #XXXXX TODO: Once this PR has been reviewed/approved.

Further comments

I wasn't entirely sure where to place the test for this, as the e2e package.json looks a bit broken (everything that's not test:browser:...), and since I needed access to the GC I just went with a separate test file, command and config. Ah, the top-level package.json has all the scripts. Went with the already pre-existing testrunner.

I chose to preserve the behavior of any custom onBeforeCommand and onAfterCommand reporter hooks. Only the internal event handler hooks do the deduplication transformation.

Reviewers: @webdriverio/project-committers

[johnp]

Short self-review; going to do some fine-tuning -> Draft.

[christian-bromann]

Thanks for taking a stab at this, added some comments.

[christian-bromann]

@johnp let me know if there is anything I can do to help.

[johnp]

@christian-bromann I think I'll be able to provide an updated version this weekend. I have a version just using a function similar to transformCommandLogResult, but the new smoke test leaks memory even with the fix, so I want to debug that first.

[johnp]

So, unfortunately, using a smoke test seems to be impossible due to setTimeout handlers leaking the same scripts in the mock tests. Based on the mention of requestBodyString, I think Nock is to blame for that.

In this screenshot you can see three test runs with 10 2MiB scripts (30 -> 71 MiB) and one test run with 20 2MiB scripts (30 -> 112 MiB).

I'll clean up, rebase and push my commits.

[johnp]

I've kept the two e2e tests for now, but reduced the number of iterations to 10. With that change, their runtime is comparable to the runtime of the (failing) smoke test (~3-4s locally).

[christian-bromann]

This is excellent work 👌 thanks a lot!

Mind backporting this to the v8 branch?

[johnp]

You're welcome. I'm glad to do some open source work for a change.

Sure, I'll backport this after I've taken a look into resolving the Nock leak on their side. I really don't see why they should hold onto finished or otherwise cleared setTimeout / setImmediate references, and the setInterval spy appears completely unused (unless it's supposed to be a public export, which it doesn't appear it is).

[christian-bromann]

Awesome, I will go ahead and merge then.

[wdio-bot]

Hey johnp 👋

Thank you for your contribution to WebdriverIO! Your pull request has been marked as an "Expensable" contribution. We've sent you an email with further instructions on how to claim your expenses from our development fund. Please make sure to check your spam folder as well. If you have any questions, feel free to reach out to us at [email protected] or in the contributing channel on Discord.

We are looking forward to more contributions from you in the future 🙌

Have a nice day,
The WebdriverIO Team 🤖