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

Cypress result shows skipped tests as pending #3092

Open
Saibamen opened this issue Jan 8, 2019 · 32 comments
Open

Cypress result shows skipped tests as pending #3092

Saibamen opened this issue Jan 8, 2019 · 32 comments
Labels
type: enhancement Requested enhancement of existing feature type: unexpected behavior User expected result, but got another

Comments

@Saibamen
Copy link
Contributor

Saibamen commented Jan 8, 2019

Cypress results (with run command) shows skipped tests as pending

Current behavior:

(Results)
  ┌──────────────────────────────────────────────────┐
  │ Tests:        4                                  │
  │ Passing:      3                                  │
  │ Failing:      0                                  │
  │ Pending:      1                                  │
  │ Skipped:      0                                  │
  │ Screenshots:  0                                  │
  │ Video:        true                               │
  │ Duration:     29 seconds                         │
  │ Spec Ran:     xxx\xxx\Page2b.spec.js │
  └──────────────────────────────────────────────────┘

Result file: (see skipped="1" in Mocha Tests)

<?xml version="1.0" encoding="UTF-8"?>
<testsuites name="Mocha Tests" time="29.914" tests="4" failures="0" skipped="1">
  <testsuite name="Root Suite" timestamp="2019-01-08T18:14:13" tests="0" failures="0" time="0">
  </testsuite>
  <testsuite name="Page2b" timestamp="2019-01-08T18:14:13" tests="1" failures="0" time="8.691">
    <testcase name="Page2b should see entered data from previous page" time="8.691" classname="should see entered data from previous page">
    </testcase>
  </testsuite>
  <testsuite name="Valid data" timestamp="2019-01-08T18:14:22" tests="2" failures="0" time="21.223">
    <testcase name="Page2b Valid data goes to next page after entering valid data" time="7.554" classname="goes to next page after entering valid data">
    </testcase>
    <testcase name="Page2b Valid data should see previously entered data when I go back from next page" time="13.669" classname="should see previously entered data when I go back from next page">
    </testcase>
  </testsuite>
  <testsuite name="Invalid data" timestamp="2019-01-08T18:14:43" tests="1" failures="0" time="0">
  </testsuite>
</testsuites>

Desired behavior:

Cypress results should show 0 pending and 1 skipped tests

Steps to reproduce: (app code and test code)

cypress.json:

    "reporter": "junit",
    "reporterOptions": {
        "mochaFile": "results/output.[hash].xml"
    },

test:

describe('Page2b', function () {
    beforeEach(() => {
        cy.fixture('correctAnswers.json').as('answers')
        cy.login()
    })

    context('Valid data', function() {
        it('goes to next page after entering valid data', function () {
            // terefere
        })

        it('should see previously entered data when I go back from next page', function () {
            // terefere
        })
    })

    context('Invalid data', function () {
        it.skip('shows required validation texts after clicking on Next link on empty form', function () {
            // terefere
        })
    })

    it('should see entered data from previous page', function () {
        // terefere
    })
})

Note

This same issue when running:

context.skip('Invalid data', function () {
        it('shows required validation texts after clicking on Next link on empty form', function () {
            // terefere
        })
    })

Versions

Cypress 3.1.4, Chrome 71, Windows 10

@jennifer-shehane jennifer-shehane added the stage: needs investigating Someone from Cypress needs to look at this label Jan 8, 2019
@chrisbreiding
Copy link
Contributor

I can see how this is confusing. It's not necessarily a bug, but a terminology issue. Cypress considers a pending test to be any of the following:

  • it.skip(), context.skip, etc
  • it('some test') (no callback)

pending tests are tests you don't plan to run and explicitly mark not to run.

A skipped test is one that you plan to run but is skipped because, for example, a beforeEach hook failed.

beforeEach(() => {
  throw new Error('whoops')
})

// these will all end up 'skipped' because while you plan to run them, the runner skips them because the beforeEach threw an error
it('a test', () => {})
it('another test', () => {})

I would agree that this is confusing because it.skip creates a pending test instead of a skipped test.

It looks like the reporter you're using reports pending tests as "skipped", which seems intuitively correct but is not correct as far as Cypress is concerned because "skipped" has a different meaning.

All that to say that Cypress isn't really reporting the wrong numbers. It's reporting them correctly as it understands them.

That said, it's certainly up for debate whether the current terminology is sufficient and whether it should change to be more intuitive. I imagine it would be a lot of work and perhaps quite difficult to do so, however.

Thoughts, @brian-mann?

@Saibamen
Copy link
Contributor Author

@chrisbreiding Incorrect.
throw new Error('something') makes test marked as failed

I want to skip test and have all other tests with "green light" - have information about how many tests from single spec file was skipped

@chrisbreiding
Copy link
Contributor

Right, my bad. The first test will be failed, the subsequent ones will be skipped.

I want to skip test and have all other tests with "green light" - have information about how many tests from single spec file was skipped

Those are pending tests as far as Cypress is concerned, but your reporter is calling them skipped.

@jennifer-shehane jennifer-shehane added type: unexpected behavior User expected result, but got another stage: proposal 💡 No work has been done of this issue and removed stage: needs investigating Someone from Cypress needs to look at this labels Jan 14, 2019
@morficus
Copy link

+1 for this being confusing.

To the untrained eye, the term "pending" implies that the test is still running or has not been ran yet (as in it is queued to run in the future). Meanwhile the term "skipped" would imply that it was not ran and will not be ran. This is especially confusing because the mocha syntax is it.skip.

My initial thoughts would be to:

option 1
rename "skipped" to "excluded" and "pending" to "skipped"

option 2
consider the things currently called "skipped" to be failed tests ... since the reason they did not run would be due to a failing hook. then rename "pending" to "skipped" and keep "pending" for things with no callbacks (or get ride of it all together)

@dsesami
Copy link

dsesami commented Jun 28, 2019

Any update on this? I really do prefer the option 1 provided by @morficus. It makes much more sense. If it's just string-swapping it's literally two lines.

@dialex
Copy link

dialex commented Jul 15, 2019

I also vote for option 1. Please, make this happen 🙏

@brian-mann
Copy link
Member

brian-mann commented Jul 15, 2019

I've written an extensive answer in our internal chat detailing the history as to how we arrived at the nomenclature we use. It's long, and complex, and we're in an awkward position because of what mocha chose to do - it's not something we can fix on our end unless we deviate from mocha completely - which would end up creating its own big can of worms - or introduce new unique test states (see below).

The problem is that we inherit all of the mocha reporters and we use the spec reporter by default. These mocha reporters are coupled to the definition of what a pending test is which mocha determines. Mocha decided to use the it.skip API design, but internally refers to them as pending tests in the reporter.

Deviating from this nomenclature would create a conflict from the default spec reporter, or other custom reporters you may use. You'd see the mocha stats summary indicating numbers that don't match up to our own summary at the end.

Because we don't control mocha reporters - there's not really much we can do about this. We'd either have to deviate from mocha's own API's - such as rewriting it.skip, or we'd have to rewrite and control every single mocha reporter and 3rd party mocha reporter (which is not going to happen).

In addition, Mocha has no concept of accurately depicting truly "skipped" tests. That is the one thing that we ourselves do - when a hook fails we skip the remaining tests in that suite. Sure - I agree we could probably rename this state to another word that would perhaps alleviate some confusion. But no matter what - we inherit Mocha's it.skip and pending state problems.

One idea we've batted around would be to expand the number of test states to help describe and more accurately model the actual runtime behavior of each test.

A more comprehensive list of test states that could be proposed:

  • Passed (a test that ran to completion including all its applicable hooks)
  • Failed (a test that failed in a hook or its own test body)
  • Pending (a test that had no implementation - i.e. no callback function)
  • Skipped (a test that was programmatically skipped via it.skip, or this.skip(), or our own programmatic API's)
  • Bypassed / Excluded / Aborted / Not Ran (a test that was not run because a hook failed in the current suite thus "bypassing/excluding/aborting" the remaining tests in the suite)

This would enable us to separate out the reason a test didn't run based on the cause.

Unfortunately, the upgrade path here would be pretty harsh, as the current definitions look like this:

  • Passed (a test that ran to completion including all its applicable hooks)
  • Failed (a test that failed in a hook or its own test body)
  • Pending (a test that had no implementation - i.e. no callback function, a test that had it.skip or this.skip())
  • Skipped (a test that was not run because a hook failed in the current suite thus "bypassing/excluding/aborting" the remaining tests in the suite)

@dsesami
Copy link

dsesami commented Jul 16, 2019

Unfortunately, the upgrade path here would be pretty harsh, as the current definitions look like this:

I think most users (me included) would have no issue dealing with the bumpiness of an upgrade in that sense as long as it was well-communicated. Personally, your proposed idea works fine for me. I do see pretty good feedback as far as GitHub thumbs up counts go 😄

@morficus
Copy link

I like the idea of expanding the number of test states available.

@krokofant
Copy link

This looks weird in the UI as well as referenced in the #5366 issue which I closed in favor of this issue.

This is the icon for a it.skip("test" ()=>{}) which I complained did not look as a skip icon.
image

@julian-sf
Copy link

julian-sf commented Dec 11, 2019

@brian-mann Can you more thoroughly explain what you mean by

the upgrade path here would be pretty harsh

Do you mean internally to Cypress development or for users (IE: a possible major version upgrade)?

Would it be possible to use a flag to opt into this new test state style so you can push the update to 3.x? You could force the new path in 4.0, which would eliminate the problem of incompatibility from a user perspective. Granted that would add more work on whoever makes the update.

@erwinbooij
Copy link

I actually had a bigger problem because of this issue.
The last test in our test run was set to skip and this causes the test run to never end. After 6 hours I had to cancel it myself when I found out it was still running (in the release pipeline)
I guess because it is seen as pending.

@pragapraga
Copy link

Is this Issue related to mochajs/mocha#1815? @Saibamen @brian-mann

@Saibamen

This comment has been minimized.

@akuznetsov-lineate-qa

This comment has been minimized.

@dialex

This comment has been minimized.

@akuznetsov-lineate-qa

This comment has been minimized.

@dialex

This comment has been minimized.

@estefafdez
Copy link

estefafdez commented May 29, 2020

Any idea how can I force a test to be marked as skipped then on the suite results? thanks!

@internSW
Copy link

image

I have issue, when record pending are not ending, but don't have error. Help me :((((

@julian-sf
Copy link

@brian-mann I know this is an old issue, but is there any change in how much effort this would be?

There's been a major version release (actually three, haha) since this was last commented on.

@vishnuprabhu-95
Copy link

Waiting

@nickpalmer
Copy link

@brian-mann Just because you based your product on mocha doesn't mean you should throw your hands up about fixing issues caused by that choice. When you add a library to your project you need to maintain that library as part of your project. If mocha doesn't do the right thing with respect to accounting test state then either work with mocha to fix it and contribute those changes, or if they don't want them, then hard fork mocha and use the fork. Correctly capturing test state accurately seems like a first responsibility of a test framework. This issue has been around for > 2 years with no progress.

@intoth3rid3
Copy link

Hi, I'm currently running cypress 7.7.0 and still have this issue.

do you plan to fix it?

@bahmutov
Copy link
Contributor

The explanation for the current test statuses: https://docs.cypress.io/guides/core-concepts/writing-and-organizing-tests#Test-statuses

@mlegait
Copy link

mlegait commented Apr 5, 2022

@bahmutov Thank you for the link. Could the use case this.skip() be added to this documentation?

@cypress-bot cypress-bot bot added stage: icebox and removed stage: proposal 💡 No work has been done of this issue labels Apr 28, 2022
@rajatt95
Copy link

Cypress - Tagging of test cases (Smoke, Sanity, BVT) ->
1. Found solution
2. https://github.com/cypress-io/cypress-grep
1. Still, the challenge is Test cases that are not supposed to execute, they go to the PENDING stage, not SKIPPED.
2. Every time, it opens the Browser and then, closes (HEADED mode)

@frnc07
Copy link

frnc07 commented Jan 9, 2023

Cypress - Tagging of test cases (Smoke, Sanity, BVT) -> 1. Found solution 2. https://github.com/cypress-io/cypress-grep 1. Still, the challenge is Test cases that are not supposed to execute, they go to the PENDING stage, not SKIPPED. 2. Every time, it opens the Browser and then, closes (HEADED mode)

Does cypress-grep work with cucumber feature files? If yes could you please share an example? If not, could do you know any alternative modules which work with cucumber feature files?

@valeraine
Copy link

Any update on this issue?

@nayanabiyani15
Copy link

Any update on this issue being fixed?

If not atleast provide more details when the test is considered as skipped and pending?

@bahmutov
Copy link
Contributor

@nayanabiyani15 https://glebbahmutov.com/blog/cypress-test-statuses/

@ivanjonas
Copy link

Open source contribution... I'm gonna drop my unique perspective here.

Problem 1, which everyone has commented on: the mismatch between code (it.skip()) and reporter output (pending & skipped).

Problem 2, which nobody has commented on: the source of the decision. IMO all of the following terms are useless because they don't convey any information about whether the action was done intentionally by a human or algorithmically by a computer:

  • "skipped"
  • "excluded"
  • "bypassed"
  • "not ran"

Only these two suggested terms imply a decision by computer:

  • "pending"
  • "aborted"

And no term at all above implies a clearly human decision.

I don't have a clear solution or suggestion, except that if this project will go through the trouble of renaming well-known terms, it should do so with maximum benefit by choosing unambiguous terms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Requested enhancement of existing feature type: unexpected behavior User expected result, but got another
Projects
None yet
Development

No branches or pull requests