Normalize Mocha tests to have realistic timings #3539

WaleedAshraf · 2018-10-29T22:22:00Z

Description of the Change

Add .slow() to tests which are normally taking longer time.

Benefits

More clear view of actual slow tests.

Possible Drawbacks

Should keep an eye on these test for later -- if they are getting even slower, then something's 🐟 !

Applicable issues

Fixes #2597
semver-patch

See also #2837

jsf-clabot · 2018-10-29T22:22:02Z

All committers have signed the CLA.

coveralls · 2018-10-29T22:33:09Z

Coverage increased (+0.07%) to 91.888% when pulling 2d19e82 on WaleedAshraf:issue/2597 into ffbcbf6 on mochajs:master.

plroebuck · 2018-10-30T14:04:15Z

Haven't reviewed the rest, but definitely revert 6b93c95. Changes to test specifications do not require any changes to "package.json".

WaleedAshraf · 2018-10-30T14:15:33Z

@plroebuck done.

plroebuck · 2018-10-30T14:51:49Z

Could you explain the process you used to come up with your slow threshold values?

WaleedAshraf · 2018-10-30T15:17:30Z

I ran tests for 3,4 times and added (~3 * average value of duration it took)

If a test takes 100ms

slow(100) would be red
slow (200) would orange/yellow
slow (300) would be green
slow (>300) would be no warning

e.g All of options.spec.js tests are taking ~250-300ms avg time,
so, slow(800) threshold is okay for them.

Also, if it's a semver-patch, then we need to update package.json with version bump.

plroebuck · 2018-10-30T19:15:11Z

Were the tests run on AppVeyor?

semver-patch annotates its ability to be merged directly to master here; the package version itself will change at next release.

WaleedAshraf · 2018-11-01T10:56:43Z

@plroebuck no. I ran them locally.
Is AppVeyor possible on local? Or maybe I can re-run the AppVeyor job here?

Got it 👍

plroebuck · 2018-11-01T18:05:35Z

@plroebuck no. I ran them locally.
Is AppVeyor possible on local? Or maybe I can re-run the AppVeyor job here?

Yeah, local timings won't be as much help. Need to run against the 80386 machines Appveyor seems to be powered by... :)

You could use timings from our existing commits. Try to get some spread between when you collect results -- certain times of day are much worse than others.

plroebuck · 2018-11-01T19:29:15Z

BTW, you are using the actual formula, right?

boneskull · 2018-11-01T19:43:10Z

this also resolves #2483... lots of dupes.

boneskull

thanks! just noticed one issue with the code.

I haven't had a chance to cross-reference the values and the real-world perf though

boneskull · 2018-11-01T19:43:53Z

test/integration/suite.spec.js

@@ -4,49 +4,53 @@ var assert = require('assert');
 var run = require('./helpers').runMocha;
 var args = [];

-describe('suite w/no callback', function() {
+describe('suit', function() {


plroebuck · 2018-11-01T19:51:38Z

#2483 already closed.

WaleedAshraf · 2018-11-02T09:02:26Z

@plroebuck ok. I'll cross check with previous builds and will update values where needed.
And yes, I used the same formula.

@boneskull good catch. Will push updates by this weekend. 👍

craigtaub · 2019-05-12T11:19:27Z

@WaleedAshraf any movement on this?

WaleedAshraf · 2019-05-13T08:04:46Z

Hi @craigtaub, Sorry for leaving it here for long.
I'll push changes in few days.

WaleedAshraf · 2019-05-16T11:15:04Z

@here, I think it's not needed now.
I check some of the last builds at https://travis-ci.org/mochajs/mocha/pull_requests
and I don't see many slow tests. Only one or two at max, which is fine.

We can close this PR with #2597 and #2837

craigtaub · 2019-05-18T17:19:14Z

Have you checked on AppVeyor?

To quote issue.

It's important to note that our tests run much more slowly on AppVeyor. That should be the baseline for timeout values--not your MacBook Pro.

WaleedAshraf · 2019-05-18T17:25:47Z

@craigtaub I did, but it's not that clear because logs are not colored and it's difficult to identify slow (red) tests.
Maybe we need to fix that also.

craigtaub · 2019-05-18T18:04:49Z

Ah yes, they don't support colours so that won't be possible.

Think you can tell its slow if over default slow value (for integrations believe 3750ms).

WaleedAshraf · 2019-09-04T23:05:17Z

Hi,
Let's get done with this.

I checked the lastest build on AppVeyor and there is only one test which hits the limit of 3750ms

regressions
    √ issue-1991: Declarations do not get cleaned up unless you set them to `null` - Memory Leak (16234ms)

But this is way too over the limit. Rest of tests are under the limit.

outsideris · 2019-09-09T03:45:21Z

I agreed to close this issue. Any thoughts?

plroebuck changed the title ~~Fixes: tagged slow tests~~ Normalize Mocha tests to have realistic timings Oct 30, 2018

WaleedAshraf force-pushed the issue/2597 branch from 6b93c95 to 829e8b3 Compare October 30, 2018 14:14

plroebuck added qa status: needs review a maintainer should (re-)review this pull request semver-patch implementation requires increase of "patch" version number; "bug fixes" labels Oct 30, 2018

plroebuck requested a review from boneskull October 30, 2018 19:16

boneskull suggested changes Nov 1, 2018

View reviewed changes

WaleedAshraf force-pushed the issue/2597 branch 2 times, most recently from a6ebb9f to 829e8b3 Compare May 16, 2019 10:17

Fixes: tagged slow tests

2d19e82

WaleedAshraf force-pushed the issue/2597 branch from 829e8b3 to 2d19e82 Compare May 16, 2019 10:27

juergba removed the status: needs review a maintainer should (re-)review this pull request label Feb 10, 2020

juergba closed this Feb 10, 2020

This was referenced Feb 10, 2020

make test timeouts closer to reality #2597

Closed

normalize test timeouts #2837

Closed

WaleedAshraf deleted the issue/2597 branch December 23, 2021 09:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normalize Mocha tests to have realistic timings #3539

Normalize Mocha tests to have realistic timings #3539

WaleedAshraf commented Oct 29, 2018 •

edited by plroebuck

Loading

jsf-clabot commented Oct 29, 2018 •

edited

Loading

coveralls commented Oct 29, 2018 •

edited

Loading

plroebuck commented Oct 30, 2018

WaleedAshraf commented Oct 30, 2018

plroebuck commented Oct 30, 2018

WaleedAshraf commented Oct 30, 2018 •

edited by plroebuck

Loading

plroebuck commented Oct 30, 2018

WaleedAshraf commented Nov 1, 2018

plroebuck commented Nov 1, 2018 •

edited

Loading

plroebuck commented Nov 1, 2018

boneskull commented Nov 1, 2018

boneskull left a comment

boneskull Nov 1, 2018

plroebuck commented Nov 1, 2018

WaleedAshraf commented Nov 2, 2018

craigtaub commented May 12, 2019

WaleedAshraf commented May 13, 2019

WaleedAshraf commented May 16, 2019 •

edited

Loading

craigtaub commented May 18, 2019

WaleedAshraf commented May 18, 2019

craigtaub commented May 18, 2019 •

edited

Loading

WaleedAshraf commented Sep 4, 2019

outsideris commented Sep 9, 2019

Normalize Mocha tests to have realistic timings #3539

Normalize Mocha tests to have realistic timings #3539

Conversation

WaleedAshraf commented Oct 29, 2018 • edited by plroebuck Loading

Description of the Change

Benefits

Possible Drawbacks

Applicable issues

jsf-clabot commented Oct 29, 2018 • edited Loading

coveralls commented Oct 29, 2018 • edited Loading

plroebuck commented Oct 30, 2018

WaleedAshraf commented Oct 30, 2018

plroebuck commented Oct 30, 2018

WaleedAshraf commented Oct 30, 2018 • edited by plroebuck Loading

plroebuck commented Oct 30, 2018

WaleedAshraf commented Nov 1, 2018

plroebuck commented Nov 1, 2018 • edited Loading

plroebuck commented Nov 1, 2018

boneskull commented Nov 1, 2018

boneskull left a comment

Choose a reason for hiding this comment

boneskull Nov 1, 2018

Choose a reason for hiding this comment

plroebuck commented Nov 1, 2018

WaleedAshraf commented Nov 2, 2018

craigtaub commented May 12, 2019

WaleedAshraf commented May 13, 2019

WaleedAshraf commented May 16, 2019 • edited Loading

craigtaub commented May 18, 2019

WaleedAshraf commented May 18, 2019

craigtaub commented May 18, 2019 • edited Loading

WaleedAshraf commented Sep 4, 2019

outsideris commented Sep 9, 2019

WaleedAshraf commented Oct 29, 2018 •

edited by plroebuck

Loading

jsf-clabot commented Oct 29, 2018 •

edited

Loading

coveralls commented Oct 29, 2018 •

edited

Loading

WaleedAshraf commented Oct 30, 2018 •

edited by plroebuck

Loading

plroebuck commented Nov 1, 2018 •

edited

Loading

WaleedAshraf commented May 16, 2019 •

edited

Loading

craigtaub commented May 18, 2019 •

edited

Loading