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

[CSR-2120] fix: passes, failed, skipped tests counting #146

Merged
merged 7 commits into from
Feb 6, 2025
Merged

Conversation

miguelangaranocurrents
Copy link
Contributor

@miguelangaranocurrents miguelangaranocurrents commented Feb 5, 2025

Description

Provide a brief description of the changes in this PR. Important to list all the changes made in overall. Describe any improvements, follow up tasks or edge cases related to this PR

This PR fixes the count of skipped tests as well as the failed and passed tests, taking into account the skipped as well.

PR Checklist

  • I performed manual tests or added tests that prove my fix is effective or that my feature works.
  • I have performed a self-review of my code.
  • I have annotated my PR with comments, particularly in hard-to-understand areas.
  • I have considered the security implications of this work.

Release Plan

  • I have added release:<service> labels to this PR

  • What is the release order of the affected services? N/A

  • What infrastructure changes are requires? N/A

Demo

Add screenshots or videos demonstrating the solution if applicable

https://www.loom.com/share/ba8d99a52f704719b38eaa9f8f000fdc?sid=a46ad995-8c9c-4e1a-b18b-6731e6e60a75

Manual Testing

Describe in steps (support it with images if needed) how to try the changes of this PR. (Even the start/setup of the services needed to try it out). If it's a bug also include reproduction steps

To reproduce:

  1. With an XML JUnit file with skipped test, execute the convert command.
  2. It will take the skipped as passed test

To test the fix:

  1. Do the first step from previous section
  2. You should see in the instance file the right stats of skipped, failed and passed tests

Use this as example XML file:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites name="NodeJS" tests="3" failures="1" errors="0" skipped="1" time="0.006700">
  <testsuite name="/test/nodejs/specs/server/transforms/product-card.spec.js"
    timestamp="2025-01-31T18:13:17.815Z" hostname="cgahafer-Mac14,10" errors="0" tests="2"
    failures="1" skipped="1" time="0.002541">
    <testcase
      name="FinancialDataTransformer &gt; _toBullets &gt; should return an array of strings"
      time="0.000276">
      <skipped />
    </testcase>
    <testcase
      name="FinancialDataTransformer &gt; _fetchFields &gt; should return empty if category is not found"
      time="0.001210">
      <failure message="A test message" type="testCodeFailure"><![CDATA[A test message
Error [ERR_TEST_FAILURE]: A test message
at new Promise (<anonymous>)
at Array.map (<anonymous>)]]></failure>
      <failure type="AssertionFailure" message="expected 631 to be below 200">
        <![CDATA[Failed 1 times.]]>
        <![CDATA[Collection JSON ID: e1e87782-7e0d-48dc-a937-3f6e1b931edc.]]>
        <![CDATA[Collection name: Tests.]]>
        <![CDATA[Request name: Currents API.]]>
        <![CDATA[Test description: Response time is less than 200ms.]]>
        <![CDATA[Error message: expected 631 to be below 200.]]>
        <![CDATA[Stacktrace: AssertionError: expected 631 to be below 200
   at Object.eval sandbox-script.js:2:1).]]>
      </failure>
    </testcase>
    <testcase
      name="FinancialDataTransformer &gt; _fetchFields &gt; should have values for category"
      time="0.000391" />
  </testsuite>
</testsuites>

@miguelangaranocurrents miguelangaranocurrents added bug Something isn't working release:package labels Feb 5, 2025
Copy link
Contributor

@twk3 twk3 left a comment

Choose a reason for hiding this comment

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

@miguelangaranocurrents this looks good except for one thing regarding the status

That one thing is that in all the frameworks we current support, skipped should be reported as pending to Currents.

I believe the jest reporter is already doing this for the testcase level, we need to do the same for the convert command.

Skipped in currents is for tests that were supposed to run, but didn't. (Sortof a cypress legacy item I think). We count these towards failures in Currents.

@miguelangaranocurrents
Copy link
Contributor Author

@miguelangaranocurrents this looks good except for one thing regarding the status

That one thing is that in all the frameworks we current support, skipped should be reported as pending to Currents.

I believe the jest reporter is already doing this for the testcase level, we need to do the same for the convert command.

Skipped in currents is for tests that were supposed to run, but didn't. (Sortof a cypress legacy item I think). We count these towards failures in Currents.

Got you, thanks for noticing that @twk3 I just changed it to be pending when there are skipped tests, it worked properly in the dashboard:
image

packages/cmd/src/services/convert/utils.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@twk3 twk3 left a comment

Choose a reason for hiding this comment

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

Thanks @miguelangaranocurrents

This looks to be working as expected

@twk3 twk3 merged commit ca0e246 into main Feb 6, 2025
4 checks passed
@twk3 twk3 deleted the fix/skipped-tests branch February 6, 2025 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants