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

Possible performance improvement: Generate messages only if assertions fail #295

Open
dhardtke opened this issue Jul 17, 2023 · 2 comments

Comments

@dhardtke
Copy link
Contributor

dhardtke commented Jul 17, 2023

Since serializing large objects can sometimes take some time it would be neat if Earl would generate messages lazily, i.e., only if an assertion fails.

For example, instead of:

https://github.com/l2beat/earl/blob/ea923158550558c46336f0a1038ad522ec62cc43/packages/earl/src/validators/basic/toBeTruthy.ts#L38C1-L45C2

let's do something like this:

export function toBeTruthy(control: Control) {
  control.assert({
    success: truthy()(control.actual),
    reason: () => `The value ${formatCompact(control.actual)} is not truthy, but it was expected to be truthy.`,
    negatedReason: () => `The value ${formatCompact(control.actual)} is truthy, but it was expected not to be truthy.`,
  })
}

I do not have benchmarks to back this claim up but I don't see why it wouldn't lead to a substantial performance gain.

@canab
Copy link
Contributor

canab commented Jul 26, 2023

I have tested earl on some large project.
It results in 15...20% slower run (comparing with should.js)

Quick testing the @dhardtke idea shows at least 50% better execution time on expect(object).toEqual(...)

CPU snapshot shows that most time is consumed by formatCompact and preapreStackTrace.
As I understand, the execution of those functions is only required if test fails. Avoiding them should result in significant performance boost.

@canab
Copy link
Contributor

canab commented Jul 27, 2023

I've created a pull request #296
Only toEqual function has been modified and synthetic test run time is reduced from 800ms to 15ms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants