cypress-real-events icon indicating copy to clipboard operation
cypress-real-events copied to clipboard

chore(commands): laid out groundwork for state tracking

Open Andarist opened this issue 3 years ago • 5 comments

This builds on top of https://github.com/dmtrKovalenko/cypress-real-events/pull/201

Andarist avatar Dec 27 '21 10:12 Andarist

My main motivation is that with test scenarios like this:

cy.realKeyDown('ShiftLeft')
cy.realClick()
cy.realKeyUp('ShiftLeft')

I would expect the click to be performed with the shift key being pressed down (e.shiftKey === true). This IMHO helps people to write tests that behave more than the user because this removes the need to pass the information about the shift key "modifier" to the realClick. So it's also less error-prone for users.

Andarist avatar Dec 27 '21 20:12 Andarist

I don't really like storing a state between commands because:

  1. It's implicit. I mean developer who doesn't know that this library works this way might be confused by the fact that previous command affects the current one
  2. It's uncontrolled. In your implementation there is no clear way to turn off this behavior
  3. It's required breaking change because can affect already existing tests.

For something like this I'd like to see something like this

cy.withRealEventsContext(() => {
  cy.realKeyDown('ShiftLeft')
  cy.realClick()
  cy.realKeyUp('ShiftLeft')
})

In this case if user wants he can do it's own test function that will wrap all the internal commands inside this function. Like

const testReal = (name, fn) => it(name, () => cy.withRealEventsContext(fn))

/// then in test

testReal("it works", () => {
    cy.realKeyDown('ShiftLeft')
    cy.realClick()
    cy.realKeyUp('ShiftLeft')
})

Naming is TBD have no preference there but I'd really like to have control over the mutations that can affect future command

dmtrKovalenko avatar Jan 02 '22 12:01 dmtrKovalenko

It's implicit. I mean developer who doesn't know that this library works this way might be confused by the fact that previous command affects the current one

I agree that this is implicit but I would argue that this actually helps developers. The very same argument can be said about the opposite - I was actually confused that this sort of stuff is not handled automatically. The consumer of this package is completely not aware of the implementation details, the intricacies of the CDP protocol etc and I think that it makes perfect sense to expect a "lock" (like a key being pressed down) to stay "locked" as long as the lock is not released. This would match the behavior of a lot of existing lock-like APIs out there. This would also match the behavior of both Puppeteer and Playwright.

It's uncontrolled. In your implementation there is no clear way to turn off this behavior

It's a valid concern but also... when would you like to turn this off? At the moment I don't find any compelling scenarios when this would actually be desirable. I expect people to write tests mimicking the user. If a particular sequence of commands cannot be performed by the user then it shouldn't be allowed by the API of this library.

It's required breaking change because can affect already existing tests.

Agreed, but is it a bad thing? Some breaking changes are expected - we can batch this together with some other work. Progress requires breaking changes from time to time - we grow and learn new things about the underlying tools here and it would be a waste not to learn from that and get stuck with the older API for an undefined amount of time, right?

Andarist avatar Jan 02 '22 12:01 Andarist

@dmtrKovalenko any more thoughts about this one?

Andarist avatar Jan 24 '22 11:01 Andarist

Again sorry for long answering but:

I agree that this is implicit but I would argue that this actually helps developers. The very same argument can be said about the opposite - I was actually confused that this sort of stuff is not handled automatically. The consumer of this package is completely not aware of the implementation details, the intricacies of the CDP protocol etc and I think that it makes perfect sense to expect a "lock" (like a key being pressed down) to stay "locked" as long as the lock is not released.

The same works opposite. I personally will expect all the commands to be completely standalone and do not imply on the other commands

It's a valid concern but also... when would you like to turn this off

I would like to avoid this behavior as much as possible and execute one command at a time. Because I know too much about how testing drivers works inside :D. But in fact yes it can confuse somebody when you execute a command and 3 commands after it affects something else.

Make sure that user can do the keydown in a shared command which is out of context

Some breaking changes are expected

I don't think this is a valid reason to make a breaking change for this type of feature

dmtrKovalenko avatar Feb 11 '22 17:02 dmtrKovalenko