sh33dafi / jest-sonar

A jest reporter that generates sonar reports
MIT License
50 stars 11 forks source link

Skipped test suite results in broken output #31

Closed tinovyatkin closed 3 years ago

tinovyatkin commented 4 years ago

Test suite skipped at too level, like this:

describe.skip('Completely skipped suite', () => {
  it('some test', () => {})
})

results in output where each of the internal tests is listed with null at duration field, as a result SonarScannel throws on that file as being invalid.

sh33dafi commented 3 years ago

Can you maybe post a complete example or test with the latest version when I have a test suite like this

`describe('description', () => { it('this is a success', () => { expect(true).toEqual(true); });

it('this is a failure', () => {
    expect(true).toEqual(false);
});

});

describe.skip('Completely skipped suite', () => { it('some test', () => {}) it('this is a success', () => { expect(true).toEqual(true); });

it('this is a failure', () => {
    expect(true).toEqual(false);
});

describe('sub test', () => {
    it('success', () => {
        expect(true).toEqual(true);
    });
});

})`

It generates a report like `

...

`

Duration is 0 in this case

sh33dafi commented 3 years ago

@tinovyatkin Can you verify you still have the same issue with the latest version?

jlestes commented 3 years ago

HI @sh33dafi I have a similar issue in my case I saw this behavior when just having a test file like this:

Basic tests from CRA:

import { render, screen } from '@testing-library/react';
import App from './App';

test('renders learn react link', () => {
  render(<App />);
  const linkElement = screen.getByText(/learn react/i);
  expect(linkElement).toBeInTheDocument();
});

test.todo('im todo')

test.skip('im skipped', () => {
  expect(true).toBeTruthy()
})

test('im failing', () => {
  expect(1).toBe(2)
})

describe('im describing', () => {
  test('in describe', () => {
    expect(true).toBeTruthy()
  })

  test.skip('skipped in describe', () => {
    expect(true).toBeTruthy()
  });

  test.todo('todo in describe');

  test('in describe failing', () => {
    expect(1).toBe(2)
  })
})

in the output coverage/sonar-report.xml I am seeing:

<testExecutions version="1">
<file path="src/App.test.js">
<testCase name="renders learn react link" duration="64" />
<testCase name="im todo" duration="null" />
<testCase name="im skipped" duration="null">
<skipped message="im skipped"/>
</testCase>
<testCase name="im failing" duration="1">
<failure message="Error"><![CDATA[Error: expect(received).toBe(expected) // Object.is equality

Expected: 2
Received: 1
    at Object.&lt;anonymous&gt; (/Users/jlestes/Code/bad-react/src/App.test.js:17:13)
    at Promise.then.completed (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/utils.js:276:28)
    at new Promise (&lt;anonymous&gt;)
    at callAsyncCircusFn (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/utils.js:216:10)
    at _callCircusTest (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/run.js:212:40)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at _runTest (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/run.js:149:3)
    at _runTestsForDescribeBlock (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/run.js:63:9)
    at run (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/run.js:25:3)
    at runAndTransformResultsToJestFormat (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:176:21)
    at jestAdapter (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:109:19)
    at runTestInternal (/Users/jlestes/Code/bad-react/node_modules/jest-runner/build/runTest.js:380:16)
    at runTest (/Users/jlestes/Code/bad-react/node_modules/jest-runner/build/runTest.js:472:34)]]></failure>
</testCase>
<testCase name="im describing in describe" duration="0" />
<testCase name="im describing skipped in describe" duration="null">
<skipped message="im describing skipped in describe"/>
</testCase>
<testCase name="im describing todo in describe" duration="null" />
<testCase name="im describing in describe failing" duration="0">
<failure message="Error"><![CDATA[Error: expect(received).toBe(expected) // Object.is equality

Expected: 2
Received: 1
    at Object.&lt;anonymous&gt; (/Users/jlestes/Code/bad-react/src/App.test.js:32:15)
    at Promise.then.completed (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/utils.js:276:28)
    at new Promise (&lt;anonymous&gt;)
    at callAsyncCircusFn (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/utils.js:216:10)
    at _callCircusTest (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/run.js:212:40)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at _runTest (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/run.js:149:3)
    at _runTestsForDescribeBlock (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/run.js:63:9)
    at _runTestsForDescribeBlock (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/run.js:57:9)
    at run (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/run.js:25:3)
    at runAndTransformResultsToJestFormat (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:176:21)
    at jestAdapter (/Users/jlestes/Code/bad-react/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:109:19)
    at runTestInternal (/Users/jlestes/Code/bad-react/node_modules/jest-runner/build/runTest.js:380:16)
    at runTest (/Users/jlestes/Code/bad-react/node_modules/jest-runner/build/runTest.js:472:34)]]></failure>
</testCase>
</file>
</testExecutions>

As you can see above the skipped and todo tests are reporting null for the duration.

I think you can make this change to the duration here ?? 0:

    toSonarReport(results) {
        const testResults = results.testResults.map(testResult => ({
            path: this.mapFilePath(testResult.testFilePath),
            testCases: testResult.testResults.map(testCase => {
                return {
                    name: testCase.fullName,
                    duration: '' + testCase.duration ?? 0,
                    failures: testCase.failureMessages,
                    status: testCase.status
                };
            })
        }));

        return this.render(testResults);
    }

My only thoughts here is that this might not work with all versions of Node, so could be done with something like a ternary or this syntax || 0

Happy to raise a PR for this if you'd like. I just need to get the repo setup locally. Thanks!

jlestes commented 3 years ago

Also to answer your question I saw this with the latest version.

jlestes commented 3 years ago

@sh33dafi Actually got this fixed with some changes and updated the tests. Also was able to verify this fix with Node v10, v8 failed with jest-cli. I will be raising a PR.

jlestes commented 3 years ago

@sh33dafi I've raised #34, hopefully all looks good on that and resolves this issue 😄

sh33dafi commented 3 years ago

@sh33dafi I've raised #34, hopefully all looks good on that and resolves this issue 😄

Hi thanks for the PR I will take a look and merge it this weekend. Thanks for your effort!