cypress-io/cypress

Proposal: .pipe command that is essentially a retryable .then

Closed this issue · 23 comments

NicholasBoll commented

Proposal: Command that is essentially a retryable .then

Example:

const getTable = id => cy.get(`[data-testid=${id}`)
const getColumnNames = $el => Array.from($el.find('[role=columnheader]')).map(el => el.innerText)

getTable('identifier')
  .then(getColumnNames) // The `should` doesn't cause this to re-evaluate
  .should('deep.equal', ['Column One', 'Column Two'])

The getTable will get an element given a test id and getColumnNames will get the text contents of a table's column headers. If the table's column headers aren't statically known (maybe column order is a user preference), the assertion immediately fails.

I don't think we want .then to retry. I think it is good that .then has strong guarantees that it will only be run once. It just makes composition of functions not quite fit.

There could be another command that does re-run if there is an assertion:

getTable('identifier')
  .pipe(getColumnNames) // The `should` would cause re-evaluation
  .should('deep.equal', ['Column One', 'Column Two'])
NicholasBoll commented

Quick and dirty (from its and invoke in connectors.coffee):

Cypress.Commands.add('pipe', { prevSubject: true }, (subject, fn) => {
  const getElements = $el => {
    if (!($el != null ? $el.length : undefined)) {
      return
    }

    // unroll the jquery object
    const els = $el.length ? Array.from($el) : $el

    if (els.length === 1) {
      return els[0]
    } else {
      return els
    }
  }

  const options = {}

  const getMessage = () => {
    return fn.name
  }

  const message = getMessage()

  options._log = Cypress.log({
    message,
    $el: Cypress._.isElement(subject) ? subject : null,
    consoleProps() {
      return { Subject: subject }
    },
  })

  const getValue = () => {
    const remoteSubject = cy.getRemotejQueryInstance(subject)

    const actualSubject = remoteSubject || subject

    const getFormattedElement = $el => {
      if (Cypress._.isElement($el)) {
        return getElements($el)
      } else {
        return $el
      }
    }

    const value = fn(actualSubject)

    if (options._log) {
      options._log.set({
        consoleProps() {
          const obj = {}

          obj.Function = message
          obj.Body = fn.toString()

          Cypress._.extend(obj, {
            On: getFormattedElement(actualSubject),
            Yielded: getFormattedElement(value),
          })

          return obj
        },
      })
    }

    return cy.isCy(value) ? actualSubject : value
  }

  // wrap retrying into its own
  // separate function
  const retryValue = () =>
    Cypress.Promise.try(getValue).catch(err => {
      options.error = err
      return cy.retry(retryValue, options)
    })
  const resolveValue = () =>
    Cypress.Promise.try(retryValue).then(value =>
      cy.verifyUpcomingAssertions(value, options, {
        onRetry: resolveValue,
      }),
    )
  return resolveValue()
})

Typedef:

declare namespace Cypress {
  interface Chainable<Subject> {
    /**
     * Enables you to work with the subject yielded from the previous command.
     * Similar to `cy.then`, except the function can be re-evaluated if followed by
     * a `.should`. Your function should not have any side effects and could be run
     * many times per second.
     * 
     * ```ts
     * cy.get('body')
     *   .pipe($body => $body.children())
     *   .should('have.length', 3)
     * ```
     */
    pipe<S>(
      fn: (this: { [key: string]: any }, currentSubject: Subject) => Chainable<S>,
      options?: Partial<{ timeout: number }>,
    ): Chainable<S>
    pipe<S extends object | any[] | string | number | boolean>(
      fn: (this: { [key: string]: any }, currentSubject: Subject) => S,
      options?: Partial<{ timeout: number }>,
    ): Chainable<S>
  }

Video:
cypress-pipe

First run I intentionally let it fail (expectation doesn't include the 3rd column). The second time I manually delete the 3rd column header and the assertion succeeds

brian-mann commented

You can do this with .should(fn)

getTable('identifier')
.should(($el) => {
  const arr = Array.from($el.find('[role=columnheader]')).map(el => el.innerText)
  expect(arr).to.deep.eq(['Column One', 'Column Two'])
})

Although I think you can further reduce this like so:

getTable('identifier')
.find('[role=columnheader]')
.invoke('text')
.should('deep.equal', ['Column One', 'Column Two'])
bahmutov commented
brian-mann commented

Ahh invoke('text') returns a string not array...

We should probably add a .map function since we already have .each... why not

You could probably invoke jquery's map too

brian-mann commented

@bahmutov

There are a lot of docs on should(fn) :-P

It has a very particular use case which is perfect this this situation.

Should represents the only need for "retryability" and using it enables you to do anything you want synchronously (such as exactly what you're doing)

NicholasBoll commented

@brian-mann .should cannot change the subject. The point of the helper is to take in some value and return some other value in the chain. In this case, take a table element and return an array of strings.

In this particular situation, the helper could contain the .should to get the desired functionality with now code change and you pasted originally. So far, we've gotten really far with just using .then to chain functions together to avoid using custom commands.

// more composable, getColumnNames can be used for more than just assertions.
getTable('identifier')
  .pipe(getColumnNames)
  .should('deep.equal', ['Column One', 'Column Two'])

// very specific
const verifyColumnNames = (columns) => (el) =>
  cy.wrap(el).should(el => {
    expect(getColumnNames(el)).to.deep.equal(columns)
  })

getTable('identifier')
  .then(verifyColumnNames(['Column One', 'Column Two']))

Both will wait.

NicholasBoll commented

The .pipe implementation only works with synchronous values. I'm not experienced enough with how Cypress works to work with $Chainers. I end up doing what is done in the .then implementation which only evaluates the function once and will wait until the timeout, but never re-evaluate the chain.

NicholasBoll commented

Also, I use TypeScript. Right now, invoke loses any type info for the subject as does its with dots. I tend to not use them in favor of .then so TypeScript can correctly pass down type information. But this looses the retryabilty of both those commands.

brian-mann commented

If we were to add a .map function which changes the subject to an array of mapped values then I think this would just work...

getTable('identifier')
.find('[role=columnheader]')
.map('text') // call $el.text() on each
.should('deep.equal', ['Column One', 'Column Two'])
NicholasBoll commented

I updated the pipe command implementation to handle a function that returns a Cypress $Chainer. It essentially does what a .then does (returns actualSubject: return cy.isCy(value) ? actualSubject : value

I'm not sure it is possible to retry Cypress chainables. In my experiments I can't get the command to wait for the value. I'm not sure it would be wise if I could. Many Cypress commands have side-effects that shouldn't be re-run.

Since the entire pipe implementation can be a custom command, I guess it is up to the Cypress team if it makes sense or if it should be an external package. For many of my team's use-cases, .pipe can simply replace .then with the benefit of re-running synchronous functions when followed by a should. We are making many application-specific helper functions that help us compose regression tests across repository boundaries.

NicholasBoll commented

@brian-mann Your example looks more like .mapInvoke. Lodash's .map has a property shorthand for accessing properties, but not invoking functions. .map would be a useful addition - assuming it can take a iteratee function it would be versatile.

The getColumnNames is actually a bit more complicated. Column headers can contain filters that would add extra cruft to .text(). Using a map function would work to get meaningful text out of the columnheader DOM node.

/**
 * Get an array of column names that are trimmed. Filters, icons, etc are ignored
 * @param $table The Table element
 *
 * ```ts
 * h.table.getTable('myTable')
 *   .then(h.table.getColumnNames)
 *   .should('deep.equal', ['Id', 'First Name', 'Last Name'])
 * ```
 */
export const getColumnNames = ($table: JQuery): string[] => {
  const $headers = $table.find('[role=columnheader]')

  return Array.from($headers)
    .map(getHeaderText)
    .filter(text => !!text)
}

/**
 * Get the text from the first child element that has text content (ignoring comment nodes).
 * This should effectively remove accidental text content from table filters
 */
const getHeaderText = (el: HTMLElement): string => {
  const text = Array.from(el.childNodes).reduce((result, node) => {
    const isCommentNode = node.nodeType === Node.COMMENT_NODE
    return isCommentNode ? result : result || (node.textContent || '').trim()
  }, '')
  return text.trim()
}

At this point, your example could work:

getTable('identifier')
  .find('[role=columnheader]')
  .map(getHeaderText)
  .should('deep.equal', ['Column One', 'Column Two'])
NicholasBoll commented

@brian-mann I think a .map makes sense. There are a few times I've wanted to use it. I can make another proposal with a .map and a reference implementation

NicholasBoll commented

Here's a library of a reference implementation: https://github.com/NicholasBoll/cypress-pipe. I'm currently trying it out to see how well it works.

brian-mann commented

@NicholasBoll how offended would the world be if we just made .map take a function or a string - and if that string was a function to invoke that with additional arguments, rather than having a separate mapInvoke function?

That would give you the ability to do either - map with property shorthand or function shorthand...

NicholasBoll commented

@brian-mann I see a few problems.

It is different than any other implementation of .map which has connotations in JavaScript: Map doesn't have side-effects - it simply returns something. For example:

[{ foo: 1, foo: 2 }].map(i => i.foo) // [1, 2]

This would make .map be a perfect candidate for retryable:

let items = [ { foo: 1 }, { foo: 2 } ]

// simulate something changing in the app over time
setTimeout(() => {
  items = [ { foo: 2 }, { foo: 3 } ]
}, 1000)

cy.wrap(items)
  .map('foo') // or .map(s => s.foo)
  .should('deep.equal', [2, 3]) // will pass after 1s
NicholasBoll commented

Sorry, didn't mean to close, but continue editing:

Lodash has .map and .invokeMap separate. I think the intent is to make it valid to return an array of functions:

_.map([ { foo: () => 'foo' }, { foo: () => 'bar' } ], s => s.foo) // [ Function, Function ]

_.invokeMap([ { foo: () => 'foo' }, { foo: () => 'bar' } ], s => s.foo) // [ 1, 2 ]

Note:
Re-reading my accidentally premature comment, I don't think you intended 'invoke' to mean no side-effects on each item, but return the value of invoking a function on each item.

brian-mann commented

Correct - I'm suggesting making invoke having a polymorphic signature that will invoke functions or return property values - the reason being that mapping jquery instances will be far more useful to call functions than property values.

const obj = { foo: 'foo', bar: (s) => 'bar' + s }

// property accessors
cy.wrap([obj, obj, obj]).map('foo').should('deep.eq', ['foo', 'foo', 'foo']) 

// function invocation with argument passing
cy.wrap([obj, obj, obj]).map('bar', 'baz').should('deep.eq', ['barbaz', 'barbaz', 'barbaz'])

This would allow you to do..

cy.get('tr').map('text').should('deep.eq', ...)
NicholasBoll commented

I think that's fair - it's a bit of extra magic that the docs can explain. The type definitions would need to be updated to use TypeScript's Conditional Types introduced in 2.8: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-8.html otherwise using .map with functions would lose type safety ('invoke' doesn't have this problem because it doesn't return anything).

What about invoking functions with side effects? I'm assuming .map would be a retryable command. It could be an issue invoking something with side-effects. I don't know if you could detect them. Maybe just the docs saying something like "Don't follow a .map that has side effects with an assertion - it will cause the side effects to be possibly invoked numerous times":

cy.get('a').map('click').should('equal', true) // would click every `a` for 4 seconds since `click` returns `undefined`

"click" is a bad example - the docs already recommend using cy.click instead of jQuery's click.

brian-mann commented

Yeah that is true. .map, .its, .invoke, and .should would retry whereas .each and .then does not.

It may seem arbitrary on the surface but I believe it makes sense... None of the above commands that retry can execute more cy commands specifically for that reason, whereas .each and .then support nested cy commands.

You are right in that if they did do arbitrary side effects those would be problematic. To date, it hasn't really been an issue.

We see people writing cy commands in a cy.should function though, but only because they are confused in general.

NicholasBoll commented

I've updated cypress-pipe after adding some nice features for debugging: https://github.com/NicholasBoll/cypress-pipe

Highlights:

  • Basically a drop-in for .then to chain helper functions together
  • Adds helper function name to the command log to help map test code to command log output
  • For functions containing Cypress commands
    • Adds before and after DOM snapshots, similar to "action" style commands like cy.click()
    • Adds Element highlights for before and after snapshots
  • For pure functions (no Cypress commands)
    • retries until there is no error
    • if an element list is returned, it will retry until the list is not empty
    • if .pipe is chained by a should, the assertion will retry until condition is met or times out
    • adds a DOM snapshot if either passed in subject or returned value is an Element

We've been using it for a while on our large SPA and it really helps compose functionality. It has helped both writing tests, but also debugging them when something goes wrong.

bahmutov commented
  • 👍2
jennifer-shehane commented

@NicholasBoll Perhaps you'd like to open a PR to add to our plugins. https://docs.cypress.io/plugins/

Can this issue be closed? Or was there some other takeaway from here to be implemented in Cypress?

NicholasBoll commented

I will add a PR to the plugins and close this PR. pipe can be fully implemented in user space.

If later the Cypress team thinks it is valuable enough, pipe could be integrated into the core.

Thanks for the feedback!