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

Add failureDetails property to circus test results #9496

Merged
merged 6 commits into from Aug 1, 2020
Merged

Add failureDetails property to circus test results #9496

merged 6 commits into from Aug 1, 2020

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented Feb 2, 2020

Summary

This is a first pass run at the second part of #8840

I used this to get the details:

class MyCustomReporter {
  onRunComplete(contexts, results) {
    console.log('Custom reporter output:');
    console.log(
      results.testResults[0].testResults.map(r =>
        r.failureDetails.filter(d => !!d).map(d => d.matcherResult)
      )
    );
  }
}

module.exports = MyCustomReporter;

This seems to be working nice, but the actual data type is interesting:

image

It's an array of what seems to be sometimes an array inside another array with just a stracktrace, sometimes it's got matcherResult - I'm sure some of it has good reason (i.e similar to why eslint takes an array of configurations for a rule rather than an object), but I imagine it'd make things easier for @segrey if we could ensure even a little bit of structure.

In saying that, I also would imagine there's only so much jest can do, since it's ultimately on the reporters to report their results.

The matchers results are definitely there:
image

Test plan

Probably to have a custom reporter that expects the results from circus to include the failureDetails property.

@G-Rath
Copy link
Contributor Author

G-Rath commented Feb 2, 2020

@SimenB this was meant to be a draft PR to get your input on how I could improve on this, but I ctrl+enter'd by mistake 😂

@codecov-io
Copy link

codecov-io commented Feb 2, 2020

Codecov Report

Merging #9496 into master will decrease coverage by 0.01%.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9496      +/-   ##
==========================================
- Coverage   65.04%   65.03%   -0.02%     
==========================================
  Files         286      286              
  Lines       12140    12144       +4     
  Branches     3008     3009       +1     
==========================================
+ Hits         7897     7898       +1     
- Misses       3605     3608       +3     
  Partials      638      638
Impacted Files Coverage Δ
...us/src/legacy-code-todo-rewrite/jestAdapterInit.ts 0% <ø> (ø) ⬆️
packages/jest-jasmine2/src/reporter.ts 59.52% <ø> (ø) ⬆️
packages/jest-circus/src/utils.ts 15.03% <25%> (+0.3%) ⬆️

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 f47ade4...1a0e232. Read the comment docs.

@SimenB
Copy link
Member

SimenB commented Feb 2, 2020

The ErrorWithStack thing you can see is what we use to get better stack traces in async matchers. See the logic here for how we decide which stack trace to use (and in general the whole #6281): https://github.com/facebook/jest/blob/ffdaa751f5356de5bbdd0a906c30f31558fa2f3a/packages/jest-circus/src/utils.ts#L312-L338

What you essentially wanna do is almost always use the first exception, as we don't care about the stack trace one (I think - if we care about the stack trace you should merge them into a single object). We don't wanna leak this implementation detail to reporters

@SimenB
Copy link
Member

SimenB commented Feb 13, 2020

@G-Rath were you able to look at ths again?

@G-Rath
Copy link
Contributor Author

G-Rath commented Feb 13, 2020

@SimenB apologies I've not had the bandwidth to jump back into this 😬

If you're wanting to fast track it, I can shuffle things around - the size & complexity of the codebase is one of the primary reasons I've not had a chance to work on it, as it requires me to book out a large chuck of time if I hope to make decent progress, so having someone who knows the codebase would be a huge help 🙂

@SimenB
Copy link
Member

SimenB commented Feb 13, 2020

No problem at all, just checking you didn't miss my response 🙂 Not sure if it would help or not, but Christoph has put together a video explaining the architecture of Jest, linked here: https://jestjs.io/docs/en/architecture

Anything in particular you're stuck on that I could help unblock?

@G-Rath
Copy link
Contributor Author

G-Rath commented Feb 14, 2020

No problem at all, just checking you didn't miss my response

All good - I should have put a reply or a reaction 😄

Anything in particular you're stuck on that I could help unblock?

I think the biggest issue I have is that there's a lot of components to jest, and testing frameworks in general, that I need to get comfortable with. I'm slowly getting a better understanding of how things work, and where to make changes, but unfortunately it's one of those things that just requires time and practice.

For this PR in particular, I think the primary issue I've had is that I don't have a clear understanding on the desired output, as I've not worked with the reporter API so can't really look at the output and say "yes this is what we want and is useful" with my usual confidence.

The other issue I've had in in trying to make a good test, which ties into the stuff above - the codebase size & components in play mean I've had a hard time nailing down the actual right way to test this change in a meaningful way.

Writing this out & re-reviewing the work has refreshed my head on this, so I'm going to try and tackle it over the next few days :)

@G-Rath
Copy link
Contributor Author

G-Rath commented Feb 15, 2020

This looks like what we want 😄:

reporter output
[
  {
    ancestorTitles: [],
    duration: 1,
    failureDetails: [
      Error: 
          at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/src/mytest.spec.ts:24:9)
          at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)
          at new Promise (<anonymous>)
          at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)
          at _callCircusTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:203:39)
          at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:140:9)
          at processTicksAndRejections (internal/process/task_queues.js:94:5)
          at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)
          at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3)
          at runAndTransformResultsToJestFormat (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:179:21),
      Error: �[2mexpect.hasAssertions()�[22m
      
      Expected �[32mat least one assertion�[39m to be called but �[31mreceived none�[39m.
          at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/setupExpectEachTestHasAssertions.ts:4:25)
          at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)
          at new Promise (<anonymous>)
          at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)
          at _callCircusHook (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:164:39)
          at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:133:11)
          at processTicksAndRejections (internal/process/task_queues.js:94:5)
          at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)
          at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3)
          at runAndTransformResultsToJestFormat (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:179:21) {
        message: '\u001b[2mexpect.hasAssertions()\u001b[22m\n' +
          '\n' +
          'Expected \u001b[32mat least one assertion\u001b[39m to be called but \u001b[31mreceived none\u001b[39m.'
      }
    ],
    failureMessages: [
      'Error: \n' +
        '    at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/src/mytest.spec.ts:24:9)\n' +
        '    at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)\n' +
        '    at new Promise (<anonymous>)\n' +
        '    at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)\n' +
        '    at _callCircusTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:203:39)\n' +
        '    at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:140:9)\n' +
        '    at processTicksAndRejections (internal/process/task_queues.js:94:5)\n' +
        '    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)\n' +
        '    at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3)\n' +
        '    at runAndTransformResultsToJestFormat (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:179:21)',
      'Error: \u001b[2mexpect.hasAssertions()\u001b[22m\n' +
        '\n' +
        'Expected \u001b[32mat least one assertion\u001b[39m to be called but \u001b[31mreceived none\u001b[39m.\n' +
        '    at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/setupExpectEachTestHasAssertions.ts:4:25)\n' +
        '    at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)\n' +
        '    at new Promise (<anonymous>)\n' +
        '    at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)\n' +
        '    at _callCircusHook (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:164:39)\n' +
        '    at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:133:11)\n' +
        '    at processTicksAndRejections (internal/process/task_queues.js:94:5)\n' +
        '    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)\n' +
        '    at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3)\n' +
        '    at runAndTransformResultsToJestFormat (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:179:21)'
    ],
    fullName: 'throws!',
    invocations: 1,
    location: null,
    numPassingAsserts: 0,
    status: 'failed',
    title: 'throws!'
  },
  {
    ancestorTitles: [],
    duration: 1,
    failureDetails: [
      JestAssertionError: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mrejects�[2m.�[22mtoThrowError�[2m()�[22m
      
      Received promise resolved instead of rejected
      Resolved to value: �[31m1�[39m
          at expect (/c/Users/G-Rath/workspace/projects-oss/jest/packages/expect/build/index.js:170:15)
          at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/src/mytest.spec.ts:28:9)
          at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)
          at new Promise (<anonymous>)
          at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)
          at _callCircusTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:203:39)
          at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:140:9)
          at processTicksAndRejections (internal/process/task_queues.js:94:5)
          at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)
          at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3) {
        matcherResult: undefined,
        message: '\u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mrejects\u001b[2m.\u001b[22mtoThrowError\u001b[2m()\u001b[22m\n' +
          '\n' +
          'Received promise resolved instead of rejected\n' +
          'Resolved to value: \u001b[31m1\u001b[39m'
      },
      Error: �[2mexpect.hasAssertions()�[22m
      
      Expected �[32mat least one assertion�[39m to be called but �[31mreceived none�[39m.
          at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/setupExpectEachTestHasAssertions.ts:4:25)
          at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)
          at new Promise (<anonymous>)
          at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)
          at _callCircusHook (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:164:39)
          at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:133:11)
          at processTicksAndRejections (internal/process/task_queues.js:94:5)
          at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)
          at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3)
          at runAndTransformResultsToJestFormat (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:179:21) {
        message: '\u001b[2mexpect.hasAssertions()\u001b[22m\n' +
          '\n' +
          'Expected \u001b[32mat least one assertion\u001b[39m to be called but \u001b[31mreceived none\u001b[39m.'
      }
    ],
    failureMessages: [
      'Error: \u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mrejects\u001b[2m.\u001b[22mtoThrowError\u001b[2m()\u001b[22m\n' +
        '\n' +
        'Received promise resolved instead of rejected\n' +
        'Resolved to value: \u001b[31m1\u001b[39m\n' +
        '    at expect (/c/Users/G-Rath/workspace/projects-oss/jest/packages/expect/build/index.js:170:15)\n' +
        '    at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/src/mytest.spec.ts:28:9)\n' +
        '    at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)\n' +
        '    at new Promise (<anonymous>)\n' +
        '    at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)\n' +
        '    at _callCircusTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:203:39)\n' +
        '    at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:140:9)\n' +
        '    at processTicksAndRejections (internal/process/task_queues.js:94:5)\n' +
        '    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)\n' +
        '    at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3)',
      'Error: \u001b[2mexpect.hasAssertions()\u001b[22m\n' +
        '\n' +
        'Expected \u001b[32mat least one assertion\u001b[39m to be called but \u001b[31mreceived none\u001b[39m.\n' +
        '    at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/setupExpectEachTestHasAssertions.ts:4:25)\n' +
        '    at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)\n' +
        '    at new Promise (<anonymous>)\n' +
        '    at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)\n' +
        '    at _callCircusHook (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:164:39)\n' +
        '    at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:133:11)\n' +
        '    at processTicksAndRejections (internal/process/task_queues.js:94:5)\n' +
        '    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)\n' +
        '    at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3)\n' +
        '    at runAndTransformResultsToJestFormat (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:179:21)'
    ],
    fullName: 'promise rejection',
    invocations: 1,
    location: null,
    numPassingAsserts: 0,
    status: 'failed',
    title: 'promise rejection'
  },
  {
    ancestorTitles: [ 'my test' ],
    duration: 7,
    failureDetails: [
      JestAssertionError: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBe�[2m(�[22m�[32mexpected�[39m�[2m) // Object.is equality�[22m
      
      Expected: �[32mfalse�[39m
      Received: �[31mtrue�[39m
          at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/src/mytest.spec.ts:3:18)
          at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)
          at new Promise (<anonymous>)
          at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)
          at _callCircusTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:203:39)
          at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:140:9)
          at processTicksAndRejections (internal/process/task_queues.js:94:5)
          at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)
          at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:80:5)
          at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3) {
        matcherResult: {
          actual: true,
          expected: false,
          message: [Function],
          name: 'toBe',
          pass: false
        }
      }
    ],
    failureMessages: [
      'Error: \u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mtoBe\u001b[2m(\u001b[22m\u001b[32mexpected\u001b[39m\u001b[2m) // Object.is equality\u001b[22m\n' +
        '\n' +
        'Expected: \u001b[32mfalse\u001b[39m\n' +
        'Received: \u001b[31mtrue\u001b[39m\n' +
        '    at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/src/mytest.spec.ts:3:18)\n' +
        '    at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)\n' +
        '    at new Promise (<anonymous>)\n' +
        '    at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)\n' +
        '    at _callCircusTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:203:39)\n' +
        '    at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:140:9)\n' +
        '    at processTicksAndRejections (internal/process/task_queues.js:94:5)\n' +
        '    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)\n' +
        '    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:80:5)\n' +
        '    at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3)'
    ],
    fullName: 'my test it passes',
    invocations: 1,
    location: null,
    numPassingAsserts: 0,
    status: 'failed',
    title: 'it passes'
  },
  {
    ancestorTitles: [ 'my test' ],
    duration: 0,
    failureDetails: [
      JestAssertionError: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBe�[2m(�[22m�[32mexpected�[39m�[2m) // Object.is equality�[22m
      
      Expected: �[32mfalse�[39m
      Received: �[31mtrue�[39m
          at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/src/mytest.spec.ts:7:18)
          at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)
          at new Promise (<anonymous>)
          at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)
          at _callCircusTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:203:39)
          at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:140:9)
          at processTicksAndRejections (internal/process/task_queues.js:94:5)
          at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)
          at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:80:5)
          at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3) {
        matcherResult: {
          actual: true,
          expected: false,
          message: [Function],
          name: 'toBe',
          pass: false
        }
      }
    ],
    failureMessages: [
      'Error: \u001b[2mexpect(\u001b[22m\u001b[31mreceived\u001b[39m\u001b[2m).\u001b[22mtoBe\u001b[2m(\u001b[22m\u001b[32mexpected\u001b[39m\u001b[2m) // Object.is equality\u001b[22m\n' +
        '\n' +
        'Expected: \u001b[32mfalse\u001b[39m\n' +
        'Received: \u001b[31mtrue\u001b[39m\n' +
        '    at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/src/mytest.spec.ts:7:18)\n' +
        '    at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)\n' +
        '    at new Promise (<anonymous>)\n' +
        '    at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)\n' +
        '    at _callCircusTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:203:39)\n' +
        '    at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:140:9)\n' +
        '    at processTicksAndRejections (internal/process/task_queues.js:94:5)\n' +
        '    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)\n' +
        '    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:80:5)\n' +
        '    at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3)'
    ],
    fullName: 'my test fails :(',
    invocations: 1,
    location: null,
    numPassingAsserts: 0,
    status: 'failed',
    title: 'fails :('
  },
  {
    ancestorTitles: [ 'my test' ],
    duration: 4,
    failureDetails: [
      JestAssertionError: �[2mexpect(�[22m�[38;5;23m�[48;5;195mreceived�[49m�[39m�[2m).�[22mtoMatchInlineSnapshot�[2m(�[22m�[38;5;90m�[48;5;225msnapshot�[49m�[39m�[2m)�[22m
      
      Snapshot name: `my test a snapshot failure 1`
      
      �[38;5;90m�[48;5;225m- Snapshot  - 1�[49m�[39m
      �[38;5;23m�[48;5;195m+ Received  + 1�[49m�[39m
      
      �[2m  Object {�[22m
      �[2m    "p1": "hello",�[22m
      �[38;5;90m�[48;5;225m-   "p2": "sunshine",�[49m�[39m
      �[38;5;23m�[48;5;195m+   "p2": "world",�[49m�[39m
      �[2m  }�[22m
          at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/src/mytest.spec.ts:14:8)
          at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)
          at new Promise (<anonymous>)
          at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)
          at _callCircusTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:203:39)
          at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:140:9)
          at processTicksAndRejections (internal/process/task_queues.js:94:5)
          at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)
          at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:80:5)
          at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3) {
        matcherResult: {
          actual: 'Object {\n  "p1": "hello",\n  "p2": "world",\n}',
          expected: 'Object {\n  "p1": "hello",\n  "p2": "sunshine",\n}',
          message: [Function],
          name: 'toMatchInlineSnapshot',
          pass: false
        }
      }
    ],
    failureMessages: [
      'Error: \u001b[2mexpect(\u001b[22m\u001b[38;5;23m\u001b[48;5;195mreceived\u001b[49m\u001b[39m\u001b[2m).\u001b[22mtoMatchInlineSnapshot\u001b[2m(\u001b[22m\u001b[38;5;90m\u001b[48;5;225msnapshot\u001b[49m\u001b[39m\u001b[2m)\u001b[22m\n' +
        '\n' +
        'Snapshot name: `my test a snapshot failure 1`\n' +
        '\n' +
        '\u001b[38;5;90m\u001b[48;5;225m- Snapshot  - 1\u001b[49m\u001b[39m\n' +
        '\u001b[38;5;23m\u001b[48;5;195m+ Received  + 1\u001b[49m\u001b[39m\n' +
        '\n' +
        '\u001b[2m  Object {\u001b[22m\n' +
        '\u001b[2m    "p1": "hello",\u001b[22m\n' +
        '\u001b[38;5;90m\u001b[48;5;225m-   "p2": "sunshine",\u001b[49m\u001b[39m\n' +
        '\u001b[38;5;23m\u001b[48;5;195m+   "p2": "world",\u001b[49m\u001b[39m\n' +
        '\u001b[2m  }\u001b[22m\n' +
        '    at Object.<anonymous> (/c/Users/G-Rath/workspace/projects-personal/terraport/test/src/mytest.spec.ts:14:8)\n' +
        '    at Promise.then.completed (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:198:28)\n' +
        '    at new Promise (<anonymous>)\n' +
        '    at callAsyncCircusFn (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/utils.js:162:10)\n' +
        '    at _callCircusTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:203:39)\n' +
        '    at _runTest (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:140:9)\n' +
        '    at processTicksAndRejections (internal/process/task_queues.js:94:5)\n' +
        '    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:54:5)\n' +
        '    at _runTestsForDescribeBlock (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:80:5)\n' +
        '    at run (/c/Users/G-Rath/workspace/projects-oss/jest/packages/jest-circus/build/run.js:23:3)'
    ],
    fullName: 'my test a snapshot failure',
    invocations: 1,
    location: null,
    numPassingAsserts: 0,
    status: 'failed',
    title: 'a snapshot failure'
  }
]

Taken using the following:

const util = require('util');
const fs = require('fs');

class MyCustomReporter {
  onRunComplete(contexts, results) {
    console.log('Custom reporter output:');
    const result = util.inspect(results.testResults[0].testResults, {
      depth: null
    });

    fs.writeFileSync('reporter-output', result);
  }
}

module.exports = MyCustomReporter;
describe('my test', () => {
  test('it passes', () => {
    expect(true).toBe(false);
  });

  it('fails :(', () => {
    expect(true).toBe(false);
  });

  test('a snapshot failure', () => {
    expect({
      p1: 'hello',
      p2: 'world'
    }).toMatchInlineSnapshot(`
      Object {
        "p1": "hello",
        "p2": "sunshine",
      }
    `);
  });
});

it('throws!', () => {
  throw new Error();
});

test('promise rejection', async () => {
  await expect(Promise.resolve(1)).rejects.toThrowError();
})

Promises don't seem to provide matcher results, so that probably needs to be implemented at some point.

@SimenB
Copy link
Member

SimenB commented Feb 16, 2020

Looks pretty good! I think we should make sure errorsDetailed matches errors, so what about this?

diff --git c/packages/jest-circus/src/utils.ts w/packages/jest-circus/src/utils.ts
index 58d14f513..b3ecc0950 100644
--- c/packages/jest-circus/src/utils.ts
+++ w/packages/jest-circus/src/utils.ts
@@ -288,7 +288,7 @@ const makeTestResults = (
     testResults.push({
       duration: test.duration,
       errors: test.errors.map(_formatError),
-      errorsDetailed: test.errors.map(_formatErrorDetails),
+      errorsDetailed: test.errors.map(_getError),
       invocations: test.invocations,
       location,
       status,
@@ -316,9 +316,9 @@ export const getTestID = (test: Circus.TestEntry): string => {
   return titles.join(' ');
 };
 
-const _formatError = (
+function _getError(
   errors?: Circus.Exception | [Circus.Exception | undefined, Circus.Exception],
-): string => {
+): Error {
   let error;
   let asyncError;
 
@@ -330,29 +330,22 @@ const _formatError = (
     asyncError = new Error();
   }
 
-  if (error) {
-    if (error.stack) {
-      return error.stack;
-    }
-    if (error.message) {
-      return error.message;
-    }
+  if (error && (error.stack || error.message)) {
+    return error;
   }
 
   asyncError.message = `thrown: ${prettyFormat(error, {maxDepth: 3})}`;
 
-  return asyncError.stack;
-};
+  return asyncError;
+}
 
-const _formatErrorDetails = (
+function _formatError(
   errors?: Circus.Exception | [Circus.Exception | undefined, Circus.Exception],
-) => {
-  if (Array.isArray(errors)) {
-    return errors[0];
-  }
+): string {
+  const error = _getError(errors);
 
-  return errors;
-};
+  return error.stack || error.message;
+}
 
 export const addErrorToEachTestUnderDescribe = (
   describeBlock: Circus.DescribeBlock,

@G-Rath
Copy link
Contributor Author

G-Rath commented Feb 16, 2020

I like it! I'll commit it right now :)

@drew-gross

This comment has been minimized.

@codecov-io
Copy link

Codecov Report

Merging #9496 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9496      +/-   ##
==========================================
- Coverage   65.04%   65.03%   -0.02%     
==========================================
  Files         286      286              
  Lines       12140    12138       -2     
  Branches     3008     3006       -2     
==========================================
- Hits         7897     7894       -3     
  Misses       3605     3605              
- Partials      638      639       +1
Impacted Files Coverage Δ
...us/src/legacy-code-todo-rewrite/jestAdapterInit.ts 0% <ø> (ø) ⬆️
packages/jest-jasmine2/src/reporter.ts 59.52% <ø> (ø) ⬆️
packages/jest-circus/src/utils.ts 14.17% <0%> (-0.56%) ⬇️
packages/expect/src/utils.ts 94.96% <0%> (-1.26%) ⬇️

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 f47ade4...42531e4. Read the comment docs.

@SimenB
Copy link
Member

SimenB commented Feb 16, 2020

@segrey does this look like something you can use?

@drew-gross

This comment has been minimized.

@segrey
Copy link

segrey commented Feb 24, 2020

@segrey does this look like something you can use?

Sorry for the delay. Yes, seems testResult.errorsDetailed can be used.
Am I right that testResult.errorsDetailed[i] contains all testResult.errors[i] data, so only testResult.errorsDetailed can be used now?
Regarding having more detailed failure information: it'd be great to have separate error name and error stack. Now testResult.errorsDetailed[i].name contains both name and stack, right?

If it's possible to build jest locally with the patch applied, I can try it out locally and let you know how it goes. Any hints?

@G-Rath
Copy link
Contributor Author

G-Rath commented Feb 24, 2020

Regarding having more detailed failure information: it'd be great to have separate error name and error stack. Now testResult.errorsDetailed[i].name contains both name and stack, right?

Depending on what you're referring to by name, maybe no? - the output is a bit confusing because Node has special logic for printing Errors, so it's easy to miss that the Error object has a matcherDetails property.

If it's possible to build jest locally with the patch applied, I can try it out locally and let you know how it goes. Any hints?

You can checkout my fork, build, and then link or copy the resulting build folders for the packages changed by this PR.

Here's the repo/code I've been using for testing these changes, complete with applied patches. Running npm install will handle everything, including applying the patches.


On an aside, it seems that the "highlight problem line in test" inspection has stopped working - I've reported it as WEB-43995.

@SimenB
Copy link
Member

SimenB commented May 2, 2020

@G-Rath We've recently fixed a memory leak in the test result (#9934), hopefully this still works even though we don't pass the objects through as-is anymore

@G-Rath
Copy link
Contributor Author

G-Rath commented May 2, 2020

Awesome, patching leaks is always good - cheers for the heads up.

Currently this is actually failing because of babel-jest:

AssertionError [ERR_ASSERTION]: Package babel-jest is missing typesVersions field

@codecov-io
Copy link

Codecov Report

Merging #9496 into master will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9496      +/-   ##
==========================================
- Coverage   64.24%   64.22%   -0.02%     
==========================================
  Files         292      292              
  Lines       12466    12465       -1     
  Branches     3078     3074       -4     
==========================================
- Hits         8009     8006       -3     
- Misses       3809     3810       +1     
- Partials      648      649       +1     
Impacted Files Coverage Δ
...us/src/legacy-code-todo-rewrite/jestAdapterInit.ts 0.00% <ø> (ø)
packages/jest-circus/src/utils.ts 14.17% <0.00%> (-0.56%) ⬇️
packages/jest-jasmine2/src/reporter.ts 58.13% <0.00%> (-1.39%) ⬇️
packages/expect/src/utils.ts 94.96% <0.00%> (-1.26%) ⬇️

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 649796d...89c1123. Read the comment docs.

@SimenB
Copy link
Member

SimenB commented May 2, 2020

@G-Rath fixed on master, sorry about that. We got a bit excited when we decided to do a new major and merged some bad merge conflict resolutions 😛

@G-Rath
Copy link
Contributor Author

G-Rath commented May 2, 2020

Ok now this is just getting silly 😂

image

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM - could you add some tests and a changelog entry?

@segrey confirmation this can help you out in WebStorm would be highly appreciated! 🙂

@G-Rath
Copy link
Contributor Author

G-Rath commented May 3, 2020

could you add some tests

ugh I was hoping past me had added one, but apparently I was hoping future me would do so.
I'll have a look at chucking my custom reporter into a set of snapshot tests, and/or maybe expanding existing tests to test these properties.

@G-Rath
Copy link
Contributor Author

G-Rath commented Jul 4, 2020

@SimenB so I've gotten around to taking a stab at implementing tests, but I'm not sure the best way to actually go about it - would you have any recommendations?

So far I've got a test that runs the tests in my sample repo using the custom matcher, and makes a snapshot of the output; while that works, the snapshot contains personal file paths in the stacktrace, which'll mean the tests will fail if run anywhere but on my local machine 😬

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

e2e/__tests__/failureDetailsProperty.test.ts Outdated Show resolved Hide resolved
e2e/__tests__/failureDetailsProperty.test.ts Outdated Show resolved Hide resolved
e2e/failureDetails-property/myreporter.js Outdated Show resolved Hide resolved
@G-Rath G-Rath requested a review from SimenB July 4, 2020 23:49
@G-Rath
Copy link
Contributor Author

G-Rath commented Jul 5, 2020

Just going to put this here for reference:

'[\\\\n' +
          '  {\\\\n' +
          '    \\"actual\\": \\"\\",\\\\n' +
          '    \\"error\\": {\\\\n' +
          '      \\"matcherResult\\": {\\\\n' +
          '        \\"actual\\": true,\\\\n' +
          '        \\"expected\\": false,\\\\n' +
          '        \\"name\\": \\"toBe\\",\\\\n' +
          '        \\"pass\\": false\\\\n' +
          '      }\\\\n' +
          '    },\\\\n' +
          '    \\"expected\\": \\"\\",\\\\n' +
          '    \\"matcherName\\": \\"\\",\\\\n' +
          '    \\"message\\": \\"Error: expect(received).toBe(expected) // Object.is equality\\\\\\\\n\\\\\\\\nExpected: false\\\\\\\\nReceived: true\\",\\\\n' +
          '    \\"passed\\": false,\\\\n' +
          '    \\"stack\\": \\"Error: expect(received).toBe(expected) // Object.is equality\\\\\\\\n\\\\\\\\nExpected: false\\\\\\\\nReceived: true\\\\\\\\n    at Object.toBe (<rootDir>/projects-oss/jest/e2e/failureDetails-property/__tests__/tests.test.js:11:18)\\\\\\\\n    at Object.asyncJestTest (<rootDir>/projects-oss/jest/packages/jest-jasmine2/build/jasmineAsyncInstall.js:100:37)\\\\\\\\n    at <rootDir>/projects-oss/jest/packages/jest-jasmine2/build/queueRunner.js:45:12\\\\\\\\n    at new Promise (<anonymous>)\\\\\\\\n    at mapper (<rootDir>/projects-oss/jest/packages/jest-jasmine2/build/queueRunner.js:28:19)\\\\\\\\n    at <rootDir>/projects-oss/jest/packages/jest-jasmine2/build/queueRunner.js:75:41\\\\\\\\n    at processTicksAndRejections (internal/process/task_queues.js:97:5)\\"\\\\n' +
          '  }\\\\n' +
          ']',

It seems that sometimes the stack trace can include processTicksAndRejections, which seems to occur only when running cold (i.e if you run the tests multiple times in the row, you won't see it) - this seems to be the main reason CI is failing, as its always present in MacOS runs 🤔

@G-Rath
Copy link
Contributor Author

G-Rath commented Jul 29, 2020

@SimenB I think this is good to go, except the tests are failing due to variations in the stack trace - any ideas on how to handle this?

There's not a consistent enough pattern that I can see to craft a string transformer that doesn't undermine the point of the snapshot test a bit, unless we're happy with just dropping the strack-trace all together?

@SimenB
Copy link
Member

SimenB commented Jul 29, 2020

Stripping out the stack in the snapshot entirely seems reasonable to me

@SimenB
Copy link
Member

SimenB commented Jul 30, 2020

@G-Rath could you resolve the conflict? Ready to land after that 🙂

@SimenB
Copy link
Member

SimenB commented Jul 30, 2020

Oh, and a changelog entry 👍

@G-Rath G-Rath requested a review from SimenB August 1, 2020 01:06
@G-Rath
Copy link
Contributor Author

G-Rath commented Aug 1, 2020

@SimenB things are looking good here - the failing check is CircleCI, and seems to be because of memory leaks that I don't think are related to this change.

@SimenB SimenB merged commit 9186361 into jestjs:master Aug 1, 2020
@SimenB
Copy link
Member

SimenB commented Aug 1, 2020

Thanks!

@SimenB
Copy link
Member

SimenB commented Aug 10, 2020

@segrey this is released in https://github.com/facebook/jest/releases/tag/v26.3.0. If you could give it a spin that'd be lovely - Jest Circus is gonna be the default in Jest 27, and while we don't a have a timeline, it'd be awesome if the integration for IDEA IDEs was good from the start 🙂

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants