feat: add long test time threshold option #9366

Draco-Umbratilis · 2020-01-05T13:21:38Z

Summary

Adds slowTestThreshold option to configure what is considered to be a long running test. Defaults to 5 seconds since it was the original hardcoded value. Based on suggestions from #7220.

Fixes #7220

Test plan

I have added two new tests to get_result_header.test.js.

codecov-io · 2020-01-05T13:34:54Z

Codecov Report

Merging #9366 into master will increase coverage by 0.02%.
The diff coverage is 50%.

@@            Coverage Diff             @@
##           master    #9366      +/-   ##
==========================================
+ Coverage   64.77%   64.79%   +0.02%     
==========================================
  Files         280      280              
  Lines       11987    11988       +1     
  Branches     2956     2955       -1     
==========================================
+ Hits         7764     7768       +4     
  Misses       3591     3591              
+ Partials      632      629       -3

Impacted Files	Coverage Δ
packages/jest-config/src/Defaults.ts	`100% <ø> (ø)`	⬆️
packages/jest-config/src/normalize.ts	`77.74% <ø> (ø)`	⬆️
packages/jest-config/src/Descriptions.ts	`100% <ø> (ø)`	⬆️
packages/jest-config/src/index.ts	`11.76% <ø> (ø)`	⬆️
packages/jest-config/src/ValidConfig.ts	`100% <ø> (ø)`	⬆️
packages/jest-test-result/src/helpers.ts	`0% <ø> (ø)`	⬆️
packages/jest-runner/src/runTest.ts	`2.17% <0%> (-0.03%)`	⬇️
packages/jest-reporters/src/get_result_header.ts	`66.66% <100%> (+19.04%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9419034...582775e. Read the comment docs.

SimenB · 2020-01-05T20:03:12Z

Thanks for the PR! Can we make this a reporter option instead?

thymikee

Let's discuss it a bit more

thymikee · 2020-01-07T12:52:21Z

packages/jest-reporters/src/get_result_header.ts

@@ -46,7 +46,7 @@ export default (
    : null;

  const testDetail = [];
-  if (runTime !== null && runTime > 5) {
+  if (runTime !== null && runTime > globalConfig.longTestThreshold) {


As OP from the mentioned issue said, such config option makes sense for specific scenarios (like running e2e tests), or in Jest nomenclature "projects". Hence, putting it into globalConfig is not the best place.

I'd go with either putting this inside projectConfig or making it an option of a reporter as @SimenB suggested. I'm not 100% convinced where the responsibility lies, but for now I'd expect Jest to determine whether the test was slow and pass that information to the reporter to render, so I'm leaning towards projectConfig solution.

Draco-Umbratilis · 2020-01-12T12:42:29Z

In hindsight globalConfig was probably the worst place to put it in. I have reworked the feature as @thymikee suggested, making the runner responsible for decision whether the test was slow and reporter just reacts to new slow flag in perfStats.

joshuapinter · 2020-07-28T13:01:04Z

Hey @Draco-Umbratilis, great job with this feature. Looks like there's some interest from @SimenB to get this merged in. Are you able to get this branch updated and pushed across the finish line, or do you want a hand with the last bit?

Lemme know as we're keen to get this merged in and used in our project, but I don't want to step on your toes here.

Thanks!

Draco-Umbratilis · 2020-07-28T13:18:57Z

Hi @joshuapinter, I will try to update the branch later today. Lets see whether someone (@SimenB @thymikee ?) will have some more notes after that or not.

joshuapinter · 2020-07-28T13:26:02Z

👍 Beauty. Looks like you've done a great job with it.

SimenB

Forgot about this! 😅 I don't feel too strongly about making it a reporter option (mostly because it's sorta finicky as we have some magic around setting default reporters which might mess up here)

packages/jest-reporters/src/get_result_header.ts

docs/Configuration.md

SimenB · 2020-07-28T13:30:26Z

packages/jest-runner/src/runTest.ts

@@ -259,7 +259,12 @@ async function runTestInternal(
      result.numPendingTests +
      result.numTodoTests;

-    result.perfStats = {end: Date.now(), start};
+    const end = Date.now();
+    result.perfStats = {


should we stick runtime or something on here as well so we don't have to do end-start in multiple places?

@Draco-Umbratilis just this comment and we should be good to go

I found one occurrence in jest-test-sequencer and made the change also there. Reflected in tests and changelog, I hope it is ok.

Yeah, I like it!

Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>

packages/jest-cli/src/init/__tests__/__snapshots__/init.test.js.snap

packages/jest-config/src/Descriptions.ts

Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>

SimenB

Thanks!

joshuapinter · 2020-07-30T19:29:32Z

Nice job @SimenB and @Draco-Umbratilis! Thank you!

github-actions · 2021-05-11T05:07:55Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

facebook-github-bot added the cla signed label Jan 5, 2020

SimenB requested review from jeysal and thymikee January 5, 2020 20:04

thymikee reviewed Jan 7, 2020

View reviewed changes

Draco-Umbratilis force-pushed the customizable-test-time-thresholds branch from d5aa73c to b0ba242 Compare January 12, 2020 12:27

Draco-Umbratilis requested a review from thymikee January 22, 2020 08:25

joshuapinter mentioned this pull request Jul 28, 2020

Custom threshold for test time warning/errors #7220

Closed

SimenB approved these changes Jul 28, 2020

View reviewed changes

Draco-Umbratilis and others added 4 commits July 28, 2020 20:34

add slow test threshold option

41d8a3f

snapshot update

91db193

Apply suggestions from code review

c727af1

Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>

post rebase fix

e2a4d28

Draco-Umbratilis force-pushed the customizable-test-time-thresholds branch from 99ad223 to e2a4d28 Compare July 28, 2020 18:59

SimenB reviewed Jul 28, 2020

View reviewed changes

packages/jest-cli/src/init/__tests__/__snapshots__/init.test.js.snap Outdated Show resolved Hide resolved

packages/jest-config/src/Descriptions.ts Outdated Show resolved Hide resolved

Draco-Umbratilis and others added 2 commits July 29, 2020 21:02

Apply suggestions from code review 2

65fdda5

Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>

code review changes

e140c48

SimenB approved these changes Jul 29, 2020

View reviewed changes

SimenB merged commit 93cde55 into jestjs:master Jul 29, 2020

sauravhiremath mentioned this pull request Jul 30, 2020

[Internal] Jest runner limitations / adoption blockers microsoft/playwright#2971

Closed

8 tasks

dargue3 mentioned this pull request Oct 24, 2020

Expose jest's slowTestThreshold option facebook/create-react-app#9883

Open

tralston mentioned this pull request Dec 13, 2020

slowTestThreshold measuring test suite time, not individual test time #10945

Closed

github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add long test time threshold option #9366

feat: add long test time threshold option #9366

Draco-Umbratilis commented Jan 5, 2020 •

edited by SimenB

Loading

codecov-io commented Jan 5, 2020 •

edited

Loading

SimenB commented Jan 5, 2020

thymikee left a comment

thymikee Jan 7, 2020

Draco-Umbratilis commented Jan 12, 2020

joshuapinter commented Jul 28, 2020

Draco-Umbratilis commented Jul 28, 2020 •

edited

Loading

joshuapinter commented Jul 28, 2020

SimenB left a comment

SimenB Jul 28, 2020

SimenB Jul 29, 2020

Draco-Umbratilis Jul 29, 2020

SimenB Jul 29, 2020

SimenB left a comment

joshuapinter commented Jul 30, 2020

github-actions bot commented May 11, 2021

feat: add long test time threshold option #9366

feat: add long test time threshold option #9366

Conversation

Draco-Umbratilis commented Jan 5, 2020 • edited by SimenB Loading

Summary

Test plan

codecov-io commented Jan 5, 2020 • edited Loading

Codecov Report

SimenB commented Jan 5, 2020

thymikee left a comment

Choose a reason for hiding this comment

thymikee Jan 7, 2020

Choose a reason for hiding this comment

Draco-Umbratilis commented Jan 12, 2020

joshuapinter commented Jul 28, 2020

Draco-Umbratilis commented Jul 28, 2020 • edited Loading

joshuapinter commented Jul 28, 2020

SimenB left a comment

Choose a reason for hiding this comment

SimenB Jul 28, 2020

Choose a reason for hiding this comment

SimenB Jul 29, 2020

Choose a reason for hiding this comment

Draco-Umbratilis Jul 29, 2020

Choose a reason for hiding this comment

SimenB Jul 29, 2020

Choose a reason for hiding this comment

SimenB left a comment

Choose a reason for hiding this comment

joshuapinter commented Jul 30, 2020

github-actions bot commented May 11, 2021

Draco-Umbratilis commented Jan 5, 2020 •

edited by SimenB

Loading

codecov-io commented Jan 5, 2020 •

edited

Loading

Draco-Umbratilis commented Jul 28, 2020 •

edited

Loading