cypress-io / cypress

Fast, easy and reliable testing for anything that runs in a browser.
https://cypress.io
MIT License
46.8k stars 3.17k forks source link

Await-ing Cypress Chains #1417

Open vacekj opened 6 years ago

vacekj commented 6 years ago

Current behavior:

await-ing cypress chains yields undefined

Desired behavior:

await-ing cypress chains yields the value of the chain (since the chain is a promise-like, it should work with await out of the box)

Additional Info (images, stack traces, etc)

I understand the recommended way to use cypress is closures, however, when dealing with multiple variables, we run into callback hell's lil' brother closure hell.

The ability to use await on Cypress chains could be beneficial in many ways:

Example code using closures

describe('Filter textbox', () => {
    beforeEach(() => {
        cy.get(test("suplovaniTable")).as("suplovaniTable");
        cy.get(test("filterTextbox")).as("filterTextbox");
    });

    it('changes data on filter text change', () => {
        cy.get("@suplovaniTable").then((el) => {
            return el[0].innerHTML;
        }).then((innerHTML) => {
            return sha256(innerHTML);
        }).then((hash) => {
            cy.get("[data-test=suplovaniTable] > tbody > :nth-child(1) > :nth-child(2)")
                .then((tds) => {
                    const text = tds[0].innerText;
                    cy.get("@filterTextbox").type(text);
                    cy.get("@suplovaniTable")
                        .then(el => el[0].innerHTML)
                        .then((innerHTML) => {
                            expect(hash).to.not.equal(sha256(innerHTML));
                        });
                });
        });
    });
});

Example code using await

describe('Filter textbox', () => {
    beforeEach(() => {
        cy.get(test("suplovaniTable")).as("suplovaniTable");
        cy.get(test("filterTextbox")).as("filterTextbox");
    });

    it('changes data on filter text change', async () => {
        const table = await cy.get("@suplovaniTable");
        const hash = sha256(table[0].innerHTML);

        const filterCell = await cy.get("[data-test=suplovaniTable] > tbody > :nth-child(1) > :nth-child(2)");
        await cy.get("@filterTextbox").type(filterCell[0].innerText);

        const newTable = await cy.get("@suplovaniTable");
        const newHash = sha256(newTable[0].innerHTML);
        expect(hash).to.not.equal(newHash);
    });
});
brian-mann commented 6 years ago

Cypress commands are not 1:1 Promises.

https://gitter.im/cypress-io/cypress?at=5a9ec8bf458cbde55701c7a8

They have promise like features in the sense that they have a .then(fn) and they can assume other thenables and do the right thing, but they themselves are not true Promises.

This has serious implications where the async/await implementation would simply not work correctly in Cypress.

What is possible - is to use async/await to replace the .then(fn) but even that has problems.

You could not use try / catch because that is the same thing as .catch() for Promises, which Cypress does not and will never have.

Because Cypress enqueues commands on a master singleton, it already manages the async coordination for you. It's impossible to ever lose a chain of commands.

What this means is - you would sometimes add the await keyword when you need to work with the yielded value of a command - but then mostly not do this.

This also means you wouldn't use the async keyword on functions that return Cypress commands.

This IMO would end up being confusing. You would use async/await inconsistently from how you'd use it outside of Cypress. It would require a lot of explanation in the docs.

FWIW you can already avoid callback hell in Cypress by writing much more terse JS. There is almost never a situation where you ever would need to create nested callbacks, or ever need to make heavy use of const.

describe('Filter textbox', () => {
  beforeEach(() => {
    cy
    .get(test('suplovaniTable')).as('suplovaniTable')
    .invoke('html')
    .then(sha256)
    .as('hash')

    cy.get(test('filterTextbox')).as('filterTextbox')
  })

  it('changes data on filter text change', () => {
    cy
    .get('@suplovaniTable')
    .find('tbody > :nth-child(1) > :nth-child(2)')
    .invoke('text')
    .then((text) => {
      cy.get('@filterTextbox').type(text)
    })

    cy
    .get('@suplovaniTable')
    .invoke('html')
    .should(function ($html) {
      expect(this.hash).to.not.equal(sha256($html))
    })
  })
})
danielkcz commented 6 years ago

It's a lot about a mindset. Being a freshman at Cypress and using async/await almost everywhere now it's tough getting used to the different paradigm for an async code.

Seeing this explanation now (and some discussion on gitter) I think it makes total sense not to support async/await. Might be worth to link this issue in docs as I believe many people will be asking for it, especially when seeing examples with .then can give a false impression that it's a great candidate for async/await.


Just to be clear from that last example. Why is there .then and .should acting like it's the same thing? I mean both are accepting callback and getting a value of the chain. I suppose that the major difference is that .should will retry till callback passes (or timeouts)? Using .then instead would work only if assertion would pass on the first run. Is that correct?

    cy
    .get('@suplovaniTable')
    .invoke('html')
    .then(function ($html) {
      expect(this.hash).to.not.equal(sha256($html))
    })

PS: I think there is a small mistake of using function instead of fatarrow as this.hash won't be available? Unless all callbacks are bound to test context internally?

NicholasBoll commented 6 years ago

PS: I think there is a small mistake of using function instead of fatarrow as this.hash won't be available? Unless all callbacks are bound to test context internally?

Yes. Mocha does this as well (as does jQuery). Much of the time it doesn't matter since people use closures instead of this.

You can see it here: https://github.com/cypress-io/cypress/blob/develop/packages/driver/src/cy/commands/connectors.coffee#L350. The type definitions also indicate the context is changed (although there isn't a good way to know what's on that context since interfaces cannot be mutated by a runtime)

brian-mann commented 6 years ago

I will open a new issue for adding a cy.alias function to avoid the use of this. I don't use this anymore in any of my code, and with fat arrow functions it'll never point to the right context anyway.

We can make cy.alias smart to point out that if you call it before an alias is defined, that it will throw a nice error explaining what you're doing wrong.

This would allow you to replace the: this.hash with cy.alias('hash') which would return a synchronous value.

brian-mann commented 6 years ago

@FredyC .then() and .should() are not the same thing. They're completely different. That's the difference - they both accept a callback function but otherwise are completely dissimilar.

Using .should() retries which insulates you from having to know the precise moment the state has settled. You can simply "describe" what you want, and Cypress will retry until it matches.

Read what I'm writing below - and then apply that to both of the cases of using .then(). Both of these uses can possibly lead to non-deterministic results that will fail your tests.


To be clear - it possible to use async/await in Cypress anytime you would otherwise use .then(fn).

However - with that said, using .then() frequently enough to need this is generally a sign of an anti pattern.

In every use case of .then() we are "locking" ourselves into something that may or may not give us a guarantee of the correct value. The whole reason we're even using .then() is to make our tests "dynamic" - but abuse of that can quickly lead to non determinism.

The root cause of the problem is documented here: https://docs.cypress.io/guides/core-concepts/conditional-testing.html#

Whenever you use a .then() if you've provided no assertions then there is no guarantees its value is even correct.

For instance you're typing text after yielding. What is the guarantee that text is correct? What if it hasn't rendered yet? What if its about to change? You'd need to add an assertion before yielding text. And if you can add an assertion about its state then you already know what the value is, and don't need to yield it.

Of course, this isn't always the case, if you're using vanilla javascript or jQuery, then you can be sure things have rendered synchronously. But try this with modern frameworks and you'll potentially be digging yourself a hole that's tough to debug.

The key to determinism is already knowing the desired result ahead of time. When that is known, you don't need to yield dynamic values - because they're not dynamic, they're static. Doing that will avoid even the need for yielding values from the DOM.

NicholasBoll commented 6 years ago

@brian-mann with cy.alias, the following would be possible, correct?

cy
  .get('@suplovaniTable')
  .invoke('html')
  .then(sha256)
  .should('not.equal', cy.alias('hash')))
brian-mann commented 6 years ago

Yes but only if its been evaluated already. It won't make anything easier - it's just a simple replacement for this and we can make it smart and throw when you attempt to use aliases which haven't been defined yet - as opposed to just returning undefined.

// nope, same problem
cy.wrap('foo').as('f')
cy.wrap('foo').should('eq', cy.alias('f')) // alias is not defined yet
// yup, works
cy.wrap('foo').as('f').then((str) => {
  expect(str).to.eq(cy.alias('f'))
})
NicholasBoll commented 6 years ago

Ah. In your example above, the .as('hash') is in the beforeEach, so it has been evaluated. That might still be a bit tricky to know when things have been evaluated.

cy.alias would have the benefit of being able to give a warning/error with a link to the documentation about this caveat. You can track the setting of items on the internal state object and issue a warning or error about cy.alias being used on a property that hasn't been set yet. A link to documentation about this in the message would help.

NicholasBoll commented 6 years ago

The following example shows the limitations of using async/await in a Cypress test:

it('should work?', async () => {
  const body1 = await cy.get('body')
  const body2 = await cy.get('body')
  cy.get('body').then(body => console.log('body', body)) // logs jQuery-wrapped body tag

  // "promises" have resolved by this point
  console.log('body1', body1) // logs jQuery-wrapped body tag
  console.log('body2', body2) // logs undefined
})
it('should work?', async () => {
  const [body1, body2] = await Promise.all([
    cy.get('body').toPromise(),
    cy.get('body').toPromise(),
  ])

  // "promises" have resolved by this point
  console.log('body1', body1) // logs jQuery-wrapped body tag
  console.log('body2', body2) // logs jQuery-wrapped body tag
})

I agree it is a bit strange especially with the prominence of async/await

I think it is best to think about Cypress commands as streams or pipelines with .then as a way to unwrap (or compose). cy.get is a common entry to this stream of data (Cypress calls them subjects). Some Cypress commands mutate DOM state (.type()), some transform the subject (.its()), but almost all return a subject for the next command in the stream. When you're done with that stream, alias it if you need it later.

danielkcz commented 6 years ago

@NicholasBoll I am curious why did you call .toPromise() in a second example? I don't even know where that API came from, don't see anywhere else. However, what happens if you do it the first example as well?

If this really is not working, then there should be some big red warning in docs about using async/await as it can only lead to headaches.

Although it's really hard to understand why is it actually different. I mean await uses .then under the hood, so why it wouldn't be able to get the value out of it? There is definitely something fishy about it.

danielkcz commented 6 years ago

On the other hand, I could imagine doing something like this.

cy.get('body').then(async ($body) => {
  const first = await doSomethingAsyncWithBody($body)
  const second = await doSomethingElseAsync(first)
  ...etc
}).alias('result')

In this case, it makes a great sense to have async/await available otherwise it would become callback hell once again. Yea sure, it can be considered edge case, but it can happen.

brian-mann commented 6 years ago

@FredyC due to the functional nature of Promises even this I disagree with. Having callback hell is always a sign of poor design.

async/await is the imperative form of Promises.

You could just as easily write it this way...

cy
.get('body')
.then(doSomethingAsyncWithBody)
.then(doSomethingElseAsync)
.as('result')
NicholasBoll commented 6 years ago

Oops, I didn't mean to leave .toPromise() in the example. I was attempting to find out how the subject was passed around and why the promise gets lost. Commands continue to get enqueued. The presence of toPromise() makes no difference.

I'm using Typescript and Promise.all doesn't consider a Cypress chainer to be a valid promise type even though it works at runtime. I did a custom command that returns a real Promise to make Promise.all happy.

NicholasBoll commented 6 years ago

@brian-mann is right. The blog post I linked explains composition of promises and how that composition applies to Cypress tests.

I tend to not use the custom Cypress Command API and just use .then(myFunction). Typescript throws a type error if myFunction gets the wrong input, which is what Cypress does at runtime with custom commands.

funkjunky commented 6 years ago

I understand for most use cases async await shouldn't be supported, however what if you wanted to slowly step through each command and do non-cypress things inbetween.

The naive example would be to console log inbetween cypress commands. Obviously you can't gaurantee your previous commands have been run, before the console.log.

If you resolved after the last operation of the stream was completed, then a programmer could work with the gaurantee.

Cypress can still be as easy and fast as possible, while giving programmers more control over the operations, without entering callback hell. (Not saying this is easy to implement, but would it not be purely beneficial?)

[side note: Reading this issue cleared up my understanding of the asyncronicity of Cypress. It's awkward that you guys neither use async, nor generators, NOR async generators, but it's also reasonable that you don't, because the way your streaming works would require an async generator and frankly most programmers would be baffled on how to work with it.]

NicholasBoll commented 6 years ago

@funkjunky It might help to know your use-case. I've learned to embrace Cypress's chaining mechanism for the declarative API Promises were meant to be.

For instance, promises allow mixing of sync and non-sync code:

cy
  .wrap('subject') // cy.wrap wraps any parameter passed into a Cypress chain
  .then(subject => {
    console.log(subject) // logs 'subject'
    return 'foo' // synchronous, but you can't do any cy commands unless you return cy.wrap('foo'). Cypress will complain about mixing sync and async code in a then
  })
  .then(subject => {
    console.log(subject) // logs 'foo'
  })
  .then(subject => {
    console.log(subject) // logs 'foo' - the previous then didn't return anything, so Cypress maintains the subject
    return Promise.resolve('bar') // You can return promises
  })
  .then(subject => {
    console.log(subject) // logs 'bar'
  })

At any point you can use .as('name') to save the subject for later use. The real gotcha is:

let foo = ''
cy.wrap('foo').then(subject => {
  foo = subject // 'foo'
})

console.log(foo) // ''
cy.wrap('').then(() => {
  console.log(foo) // 'foo'
})

This is why the Cypress docs don't recommend using let or const. The previous could be written differently:

cy.wrap('foo').as('foo')
// something stuff
cy.get('@foo').then(subject => {
  console.log(subject) // 'foo'
})

.as can be used to prevent callback hell

NicholasBoll commented 6 years ago

@funkjunky @FredyC I made a library to do this: https://www.npmjs.com/package/cypress-promise

I wouldn't recommend it for elements, but it works great for other subject types. I've been using it for a couple weeks now and it seems to be working well and is stable.

The readme has lots of examples. You can either import the function and pass any Cypress chain as an input, or you can register it to attach it to the cy object. I've done both and feel the latter is more natural.

EirikBirkeland commented 6 years ago

@NicholasBoll That is very cool, I may try it once I muster the courage (have had issues w/ Cypress promises lately). I wonder if your lib can be included in Cypress itself natively? If you think that's possible, It'd be awesome to see an issue or PR. Anyway, thanks a lot for sharing!

brian-mann commented 6 years ago

We recently tried out adding async/await to Cypress and it is theoretically possible to add - however it will almost certainly be grounds for more confusion and cause separate issues.

We are strongly considering rewriting the command enqueueing algorithm to less asynchronous and more stream-like by preventing chains of commands from resolving independently of each other. Instead they will act as a function pipeline where one receives the subject of the previous synchronously.

Even when we reach the end of the command chain, we will begin calling the next chain synchronously instead of asynchronously.

Why? Because the DOM may change in between the effective nextTick calls - and that means testing is less deterministic. The only way to make it truly deterministic is to remove any artificial asynchronousity that can potentially introduce flake.

Why does this matter to async/await? Because it will forcibly introduce async code where we are trying to prevent it at all costs. We've written our own thenable implementation to make the API familiar to people but we sacrifice ergonomics for determinism, which is the entire point of writing tests and using Cypress. I don't believe its worth the trade-off.

I'm considering trying to shoehorn async/await into here because I want to design an API that is familiar to people, because it takes so much explanation to try to change their behavior away from doing something they want to do, but don't understand why it's not a good idea to do this in the first place. They think Cypress should behave the same way "as every other JS framework" which is fine, but because of Cypress's architecture it is actually like asking it to give up its sole differentiating power to make this happen.

To make matters worse - the await keyword will not even be used in most cypress chains unless you want to yield the subject. So even if we shoehorn it in, it will not behave the same as other frameworks.

Example:

// nope no await
cy.visit()

// yup we get the $btn
const $btn = await cy.get('form').find('button').contains('Save').click()

const t = $btn.text()

// nope no await since we don't want to yield anything
cy.get('input').type('asdf')

This will be perfectly valid syntax in Cypress and yet it makes no sense. Why would we only have to await some things and not others? The above code is equivalent to the bottom code...

cy.visit()
cy.get('form').find('button').contains('Save').click().then(($btn) => {
  const t = $btn.text()

  cy.get('input').type('asdf')

Well it's because Cypress handles the enqueuing for you because it has to and you only need to grab references to objects once it's 100% sure its passed all assertions and is stable to hand off to you - otherwise it's impossible to enqueue things out of order or fire off things before they are ready. This adds a level of guarantee but it's unlike how async/await would normally work.

andreiucm commented 6 years ago

@brian-mann In my opinion async/await is more like for setup scenario. And I wan to add my 5 cents...

Usually projects already have stuff that setup scenario like the user creation, setting email .... so any project might have already code that setup some stuff for tests. But maybe I am wrong in how I am thinking and you can help me.

In our project, we should test the app every time in a new domain. So basically to create the scenario we should do a couple of request to the backend and create a domain, a user and stuff like that. We have a folder with classes that already do that. So from Cypress tests, we just should call those

When the scenario is created we visit the page. But In production our app take the domain name from the URL, unfortunately in tests it is not possible. So basically I hack the window object

let domainName=""
it("create domain", () => {
    createDomain().then(data => domainName=data.domainName)
})

cy.visit("", {onBeforLoad: (win) => { win.domainName = domainName } })

I don't care so much regarding await cy.get.... but code get much cleaner with await for scenario where I should do:

let user:any
let domain: any
User.createRandomUser()
.then(rawJson => {
   user = rawJson.data.user
   return Domain.createRandomDomain()
})
.then(rawJson => {
   domain = rawJson.data.domain
   return cy.fixture("imageUrls")
})
.then(images => {
   return User.addPictureToUser(images.url1)
})
.then(pictureId => user.pictureId = pictureId)

In this case async/await is really helpful you can't say else

                                          VS
let user:any
let domain: any
user = (await User.createRandomUser()).data.user
doman = ( await Domain.createRandomDomain()).data.domain
images = await cy.fixture("imageUrls")
imageId = await User.addPictureToUser(images.url1)
user.pictureId = pictureId

Maybe I am wrong but I think the "stumbling block" of every test framework is the test scenario since nowbody want to rewrite the code they already have. But maybe I am miss something, what do you sugest?

Anothe point is the unit tests. I have read that Cypress want to suport that. IF somebody will write code with async/awain this mean Cypress will be unable to make unit tests for this kind of code?

andreiucm commented 6 years ago

@brian-mann In a way to avoid the usage of const/let as you suggest here (https://github.com/cypress-io/cypress/issues/1417#issuecomment-370860080) I found the wrap/invoke methods, but I get even more confounded :(

        cy.wrap({ createDomain: Domain.createDomain })
            .invoke("createDomain")
            .as("domain");

        cy.get("@domain")
            .wrap({ crateUser: User.createUser }) // confusion how to get the domain without const/let???
            .invoke("crateUser", domain.name)
            .as("user");

how it is suposed to get domain for the next method in the chain for using it inside createUser ? I guess there should be a solution based on the chain "philosophy" but I can see it?

brian-mann commented 6 years ago

@andreiucm you just nest the .thens() inside of the previous so that you have a closure around the variables you want to use. It's not anymore complicated than that. Hoisting using let is a common antipattern and it's unnecessary. That creates mutable state which is always less preferred than functional patterns.

Your first example doesn't really make a lot of sense to me because you're not using the domain variable anywhere. You do use the user... but this code really below doesn't have much to do with Cypress since I'm assuming these other functions are async returning utilities not going through Cypress commands...

return User.createRandomUser()
.then(res => {
   const user = res.data.user

   return Domain.createRandomDomain()
   .then(res => {
     // ?? no use here
     const domain = res.data.domain

     return cy.fixture('imageUrls')
   })
   .then(images => {
      return User.addPictureToUser(images.url1)
   })
   .then(pictureId => user.pictureId = pictureId)
})

Your other example...

return Domain.createDomain()
.then((domain) => {
  return cy.wrap(domain).as('domain')
})

...somewhere else in your code...

cy.get("@domain")
.then((domain) => {
  return User.createUser(domain.name)
})
.then((user) => {
  return cy.wrap(user).as('user')
})

Really the problem is that you're mixing two different patterns - cypress commands and async returning utility functions. If the utility functions are just abstractions around Cypress commands you don't have to synchronize all of the async logic... but if they are then you'll need to interop between the two patterns.

Have you read this guide? https://docs.cypress.io/guides/core-concepts/introduction-to-cypress.html#Commands-Are-Asynchronous

brian-mann commented 6 years ago

@andreiucm since async/await is literally just sugar on top of Promises, there is no actual functional gain to utilizing that pattern -> it is a direct and straight 1:1 transpilation over Promises. Whatever you can do with async/await you can do with promises.

I get that the one single advantage is that it prevents nesting .then() callbacks. That is a real advantage. However it will never really "fit" within the Cypress command ecosystem because as mentioned before that cypress commands themselves are not true promises. They are promise like but more akin to steams. There is no way to interop these things correctly. Doing so will lead to nondeterminism and flakiness.

We can allow that pattern but we'll be actively fighting against what the browser transpiles it into since we are essentially trying to prevent having commands forcibly run on the nextTick / microtask.

andreiucm commented 6 years ago

I understand your problem with async/await but to be honest this nesting .then blocks are really annoying

I have read the https://docs.cypress.io/guides/core-concepts/introduction-to-cypress.html#Commands-Are-Asynchronous but let me ask again If I have some asynchronous function that doesn't use any Cypress commands like cy.request (I use superagent library to make the request to backend). Do this mean next code is wrong?

import {createDomain} from "../../..my-core/createDomain"
describe("..........
let domainName
it(" create domain and land on the page", () => {
     createDomain()
          .then(dn => domainName = dn)
     cy.visit("", {onBeforeLand:(win)=>{win.domainName=domainName}})
})

and to work should be

import {createDomain} from "../../..my-core/createDomain"
descrive("..........
it(" create domain and land on the page", () => {
     createDomain()
          .then(dn => {
               cy.visit("", {onBeforeLand:(win)=>{win.domainName=dn}})
           })
})

Note: this code is still wrong since Cypress doesn't wait for the promise createDomain to be resolved so to make it work we should switch to

it("....
     return createDomain(...

but if we do that the in console Cypress display a warning saying something like we mess cypress commands with asynchronous code!

Maybe is there a say how to wrap the external asynchronous code inside Cypress commands or how to deal when we have asynchronous function and cy. operation together?

yannicklerestif commented 5 years ago

+1 Protractor / Selenium are deprecating their old specflow (that works just like Cypress) and replacing it with a native promise based one. In my previous assignment, we migrated our tests to use async / await and really makes things way clearer. Using Cypress now and it's clearly an improvement over protractor / selenium on many topics, but this is a big disappointment.

jorganot commented 5 years ago

I have a similar question about async / await. I'm currently writing tests with Cypress for an "addEvent()" wrapper for addEventListener() that handles custom events (triple-clicks, in this case), but await doesn't seem to behave quite the same as .then() here.

  it('should handle triple-clicks', async () => {
    const el = document.createElement('div');
    const listener = cy.spy();

    addEvent(el, 'tplclick', listener);
    el.click();
    el.click();
    el.click();
    expect(listener).to.have.callCount(1);

    cy.log('should NOT trigger if clicks take longer than CLICK_SPEED');
    listener.reset();
    el.click();
    el.click();

    // This does not work as expected; callCount is 1, which indicates no delay before click()
    // await cy.wait(CLICK_SPEED + 100);
    // el.click();
    // expect(listener).to.have.callCount(0);

    // This works as expected; callCount is 0
    cy.wait(CLICK_SPEED + 100).then(() => {
      el.click();
      expect(listener).to.have.callCount(0);
    });
  });

Can someone explain why the commented out block of code there doesn't work as expected? Evidently, cy.wait() does not do something like this: const wait = ms => new Promise(r => setTimeout(r, ms));.

Btw, I used this real-life example instead of a more simplified test case to provide what I think is a legitimate use-case for wanting to use await and wait(). If there's a better way to accomplish this here, I'm all ears.

panneerselvamc commented 5 years ago

This Function returns nothing when i call this function kindly help me to overcome this

const search=()=>{
  cy.get('body').then(async ($body)=>{
    let flag =$body.text().includes("LOGIN")

    if (flag) {
      cy.log("in if")
      return(false)
    } else {
      cy.wait(4000)
      cy.log("else")
      return(true)
    }
  })
}
ollie-o commented 5 years ago

FWIW you can already avoid callback hell in Cypress by writing much more terse JS. There is almost never a situation where you ever would need to create nested callbacks, or ever need to make heavy use of const.

@brian-mann thank you for all your explanations in this thread. How would you simplify code like this, without using async/await?

I'm a beginner at Cypress, so there may be an obvious cy function I could use here.

Cypress.Commands.add(
  'upload',
  { prevSubject: 'element' },
  (subject, file, fileName, mimeType) => {
    return cy.window().then(window => {
      return Cypress.Blob.base64StringToBlob(file, mimeType).then(blob => {
        const testFile = new window.File([blob], fileName, { type: mimeType });
        cy.wrap(subject).trigger('drop', {
          dataTransfer: {
            files: [testFile],
            types: ['Files'],
          },
        });
        return new Cypress.Promise(resolve => resolve());
      });
    });
  }
);

From abramenal/cypress-file-upload

SethDavenport commented 5 years ago

not-really-promises/weird asynchrony are my biggest peeve unfortunately, generally the thing that makes me abandon a tool faster than anything else. ES6 promises are now part of the spec. Adding more leaky abstractions over asynchrony just confuses everyone even more IMO.

bahmutov commented 5 years ago

Just to add another couple of reasons why Cypress does what it does via a command queue:

it('changes the URL when "awesome" is clicked', () => {
  const user = cy

  user.visit('/my/resource/path')

  user.get('.awesome-selector')
    .click()

  user.url()
    .should('include', 
            '/my/resource/path#awesomeness')
})

I really like this mental modal. A human user would naturally wait for the page to load before clicking on a button. A human user would naturally look at the url after clicking and see if it already matches the expected value, and if not - wait a little bit longer before deciding that the test has failed. Adding "await" is just an implementation detail and unnecessary one. And most importantly - a human user would only do 1 action at a time, making race conditions that Promise.all brings really dangerous.

I have spoken about this syntax at ReactiveConf in 2018, find slides and the video here https://slides.com/bahmutov/reactive-conf

dwelle commented 5 years ago

But .then() callbacks aren't retried either, so these two are functionally identical:

beforeEach(() => {
    cy.document().then( doc => {
        doc.body.innerHTML = '<one></one>';
        setTimeout(() => doc.body.innerHTML = '<one><two></two></one>', 1000 );
    });
});

it('(1)', () => {
    cy.get('one')
    // will not be retried
    .then( $el => {
        cy.wrap($el).find('two');
    });
});

it('(2)', async () => {
    // obviously, one could just do `Cypress.$('one')` here
    const $el = await cy.now('get', 'one');
    assert.ok( $el.find('two').length );
});

For retryability, you'd have to do:

it('(3)', () => {
    cy.get('one')
    // will be retried
    .should( $el => {
        assert.ok( $el.find('two').length );
    });
});

Unless I'm missing something (I probably am), cypress doesn't backtrack and retry command chains (meaning top-level cy commands) which have already been settled --- if that's true then cypress can just resolve a command only after all its chained assertions pass. But there are probably other considerations (@bahmutov e.g. not sure why cypress needs to build an immutable chain of commands --- EDIT: apart from showing the command log up front).

(I haven't yet delved into RxJS, and how cypress uses it, so I'm most likely missing a key benefit).

Thus, I'd say the main(and arguably pretty big) downside of supporting async/await is it'd promote writing flaky code which will never be retried --- but would allow those who know what they're doing to write more streamlined code.

But, as it stands, current behavior doesn't prevent you from doing anything that you'd be able to accomplish with async/await (as already stated several comments above) --- shallow .then callbacks coupled with top-level cy.then(() => {}) for accessing intermediate state is all you need, so no async/await isn't a deal breaker.

SethDavenport commented 5 years ago

Whether it's a stream under the hood or not, people are still mostly writing tests like

Conceptually these are command/response pairs, not streams, in the mind of the person writing the tests.

If we truly want to shift the test writer's mental modal away from request/response pairs and towards streams, then sure, RxJs would make a lot of sense. But that seems very counter-intuitive to me as someone writing the tests. As commented above, that's not how humans think about using browsers.

When all is said and done, if promises or streams really aren't a good fit, then I think my biggest issue honestly is calling it .then when it's not actually a promise. Promises are hard enough for newcomers to JS to grok without other libs adding systems that look like promises but aren't.

Even in 2019 I still spend far too much time undoing the damage caused to junior developers' mental model of the event loop by non-standard, promisy-looking things things like AngularJS's $q/$scope.$digest.

At the end of the day though, cypress is the first e2e tool that we've had any success with at all, so kudos are in order. It's a great tool.

NicholasBoll commented 5 years ago

When all is said and done, if promises or streams really aren't a good fit, then I think my biggest issue honestly is calling it .then when it's not actually a promise. Promises are hard enough for newcomers to JS to grok without other libs adding systems that look like promises but aren't.

I think you nailed the heart of the issue. I think if the Cypress API was created after async/await, Cypress wouldn't have a spec incompatible then method. I think that's what trips up more veteran developers.

then is even more nuanced when you don't return a value. The docs say it just returns the original subject, but that's not true - it returns the last wrapped subject. We have a rule that you must always return something so we don't run into this edge case.

Cypress is perfectly happy if you add to the queue at a later time. It is possible for the then method to be changed to return a real promise (no longer a Cypress chain - backwards breaking change).

danielkcz commented 5 years ago

@NicholasBoll What about going other way around and introduce an alias to then and possibly deprecating that one? I think that it would be more clear that there are no promises underneath and nobody would be considering to use async/await there. Some possible aliases could be later or after.

maple-leaf commented 5 years ago

I think the most improvement for this will be make expect works correctly when previous is a cy command. Let's say we have a button on page to increase window.count when click happens.

it('increase test', () => {
expect(window.count).to.equal(1);
cy.get('button').click();
expect(window.count).to.equal(2); // <- this will fail because it execute immediately, and then click
});

For now, you have to write expect inside then

NicholasBoll commented 5 years ago

@maple-leaf window is the Cypress window, not your app's window. Why not do the following instead?

it('increase test', () => {
  cy.window().its('count').should('equal', 1);
  cy.get('button').click();
  cy.window().its('count').should('equal', 2);
});
maple-leaf commented 5 years ago

@NicholasBoll yes, the window I write is return from cy.window() at before. For this simple example, I can do this, but for some case, it's still unconvenient. For example, is there a way to optimize?

describe('test cell toolbar', () => {
    let editor, removeCb, applyCb;
    before(() => {
        cy.visitCollage().then(e => { // custom command return window.editor
            editor = e;

            // event stub
            const cb = {
                remove() {},
                apply() {},
            };
            removeCb = cy.spy(cb, 'remove');
            applyCb = cy.spy(cb, 'apply');
            editor.$events.$on('editor.collage.cell.remove', cb.remove);
            editor.$events.$on('editor.edit.apply', cb.apply);
        });

        cy.loadTpl('collage'); // do some preparation
    })

    it('click on remove button should emit event', () => {
        cy.get('.editor-toolbar-cell .icon-remove')
            .click()
            .then(() => { // this then seems not way to optimize
                expect(removeCb).to.be.called;
                const [ model, subModel ] = removeCb.getCall(0).args;
                expect(model, editor.currentElement);
                expect(subModel, editor.currentCell);

                expect(applyCb).to.be.called;
                const model2 = applyCb.getCall(0).args;
                expect(model2, editor.currentElement);
            });
    });
});

And if target I want to check is at deep level inside window or something else, it will be a long typing for its chain.

NicholasBoll commented 5 years ago

@maple-leaf I don't use Cypress for unit-style tests, so I don't tend to have stubs. I have Cypress tests for components that use example that pass in callbacks with DOM-observable side-effects.

But your test code can be simplified. I took a wild guess as to what expect(model, editor.currentElement) was intended to do. The second parameter in expect is a failure message, but no assertion was made. Perhaps it meant expect(model).to.equal(editor.currentElement)?

describe('test cell toolbar', () => {
  let removeCb, applyCb;

  // perhaps we should use a beforeEach instead? The stubs are reused for each test in a `before`
  before(() => {
      removeCb = cy.stub()
      applyCb = cy.stub()
      cy.visitCollage().then(editor => { // custom command return window.editor

          editor.$events.$on('editor.collage.cell.remove', removeCb);
          editor.$events.$on('editor.edit.apply', applyCb);

          return editor
      }).as('editor'); // save as an alias for later

      cy.loadTpl('collage'); // do some preparation
  })

  it('click on remove button should emit event', () => {
      cy.get('.editor-toolbar-cell .icon-remove')
          .click()

      cy.get('@editor')
          .should((editor) => {
              expect(removeCb).to.be.calledWith([editor.currentElement, editor.currentCell]);
              expect(applyCb).to.be.calledWith([editor.currentElement])
          });
  });
});

A .should is a good place to have expect calls. I used a Cypress alias to save editor. The assertions are simplified to test callbacks are fired with the intended arguments (making an assumption on the intent of those odd expect calls)

maple-leaf commented 5 years ago

@NicholasBoll Sorry for the wrong expect code, it should what you guess. And thanks for giving a nice way to improve this, It is clean and acceptable. Maybe I should think and write code in cypress way more often instead of jest way.

maple-leaf commented 5 years ago

@NicholasBoll It seems that editor alias not working if defined in before. cy.get in your example will fail, but if I add cy.wrap({}).as('editor') inside it or beforeEach will make cy.get('@editor') work

NicholasBoll commented 5 years ago

@maple-leaf I think that is a good take away. Unit tests and UI tests (end to end or even browser tests against component documentation) are fundamentally different. The Cypress API was designed specifically for the latter - the Cypress API tries to deal with the asynchronous nature of a browser. It waits for animations and some XHR, it retries DOM selectors until they return at least on Element, it retries should calls until the assertions pass - all transparently to you so you can focus on how a user of an app would interact.

I found the closer the tests are to a perspective of a real person, the easier the tests are to understand and maintain. We shortcut a few things like logging in or creating domain objects using REST calls.

Cypress code is simpler without too much knowledge of the internals of your app. We only mock external interfaces (like REST calls) and we create application helpers to for higher level abstractions for components we create that have interactions that are very imperative (like selecting an item from a drop-down that uses a virtual list)

maple-leaf commented 5 years ago

@NicholasBoll Yes, and I agree with that, Cypress does well job on that. But since cypress is trying to enclose the unit test, maybe provide a better way to doing this will make a good benefit. Instead of await, I recently think maybe provide a cy.eval to eval a express in cypress queue will make sense, and maybe this is a good way to approach.

For now, there's a cy.wrap will do normal object warping, and I tried to do something like cy.wrap(currentCell = editor.currentCell), which will execute the expression sync instead of later in cypress queue, and this will fail of course.

Is it hard to get expression context for cypress to execute it later? If not, maybe we can try something like this.

bahmutov commented 5 years ago

Shh don’t tell anyone I told you this but you can use “cy.now(‘command name’, ...args)” command that returns a promise and executes outside of the normal command chain. Use at your own risk though

Sent from my iPhone

On Mar 21, 2019, at 22:27, maple-leaf notifications@github.com wrote:

@NicholasBoll Yes, and I agree with that, Cypress does well job on that. But since cypress is trying to enclose the unit test, maybe provide a better way to doing this will make a good benefit. Instead of await, I recently think maybe provide a cy.eval to eval a express in cypress queue will make sense, and maybe this is a good way to approach.

For now, there's a cy.wrap will do normal object warping, and I tried to do something like cy.wrap(currentCell = editor.currentCell), which will execute the expression sync instead of later in cypress queue, and this will fail of course.

Is it hard to get expression context for cypress to execute it later? If not, maybe we can try something like this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

maple-leaf commented 5 years ago

Don't know how to use cy.now to solve problem, but seems like that I think the problem overhead. I can use custom command to wrap and execute a function to push expression into cypress queue.

    it('remove collage by delete/backspace key', () => {
        cy.get('body').type('{backspace}');
        cy.get('.editor-element-collage').click();

        expect(editor.options.collageRemovable).to.be.false;
        cy.get('body').type('{backspace}');
        cy.get('.editor-element-collage');

        cy.eval(() => editor.options.collageRemovable = true); // <- magic ~v~

        cy.get('body').type('{backspace}');
        cy.get('.editor-element-collage').should('not.exist');
    });
Cypress.Commands.add('eval', (fn) => {
    fn();
});
dwelle commented 5 years ago

@maple-leaf you can use top-level cy.then( func ) instead of your custom command.

maple-leaf commented 5 years ago

@dwelle thanks, will try that!

yordis commented 5 years ago

I am curious to know how you could avoid the wrapped case for this situation.

context('when creating a new thing', function() {
    let rowsCount

    before(function() {
      cy.get('#myid tr').then(function(elements) {
        rowsCount = elements.length
      })
      createAThing()
    });

    it('verifies presence of the new thing', function () {
      cy.get('#myid tr').should('have.length', rowsCount + 1)
    })
  })

I just need to verify that a new thing was appended to the page, but I would like to avoid having that .then there or use async/await for this

NicholasBoll commented 5 years ago

@yordis There are a few approaches to avoiding comparisons like this.

The first is to control your data ahead of time:

  context('when creating a new thing', function() {

    before(function() {
      cy.get('#myid tr').should('have.length', 0)
      createAThing()
    })

    it('verifies presence of the new thing', function () {
      cy.get('#myid tr').should('have.length', 1)
    })
  })

The other is to look for specific changes you expect (works if you don't control previous things, can generate unique IDs if necessary):

  context('when creating a new thing', function() {

    before(function() {
      createAThing('My Thing')
    })

    it('verifies presence of the new thing', function () {
      cy.get('#myid tr').should('contain', 'My Thing')
      // OR
      cy.get('#myid tr:contains("My Thing")').should('exist')
    })
  })

If you absolutely must compare the number of selectors matches to a previous number (not stable for parallel tests if you already can't control previous data since another test could be adding a thing between comparisons), is occasionally storing a variable that bad? Out of hundreds of Cypress tests I've written, I haven't needed to store variables very often. There is almost always a better, more stable, and parallel-safe way to test the UI.

yordis commented 5 years ago

Out of hundreds of Cypress tests I've written, I haven't needed to store variables very often. There is almost always a better, more stable, and parallel-safe way to test the UI.

Please help me then because I do not understand how you could avoid this from your response, so probably I need to learn something about it.

The problem for me is that I don't know the original total of the rows since this could be dynamically controlled from the backend and depending what is in the database it could be 1 or 100.

About the second example, unless I know the ID of thing I created I don't think it will work thou since My Thing will be added many times based on how many times we called the test 😬

So in this test, we are just making sure that we added the new thing, we are not worried about anything else other than going from A to B, which it means, we have a new thing in the table.

NicholasBoll commented 5 years ago

We had a similar scenario where we wanted to test that creating something new showed up in the UI. We didn't rely on the count because we wanted everything to run in parallel and it could be possible where two parallel tests both add a Thing at the same time and so the count could raise by 2 instead of 1 and the test would fail.

Instead what we did was generate a name with a uuid.

That looks like this:

const uuid = require ('uuid/v4')

  context('when creating a new thing', function() {
    const uid = uuid()
    before(function() {
      createAThing(`My Thing ${uid}`)
    })

    it('verifies presence of the new thing', function () {
      cy.get('#myid tr').should('contain', 'My Thing')
      // OR
      cy.get(`#myid tr:contains("My Thing ${uid}")`).should('exist')
    })
  })

Note the backtick for string interpolation.