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

The its command doesn't accept a log option (logging can't be disabled) #1450

Closed
cwohlman opened this issue Mar 12, 2018 · 14 comments · Fixed by #5519
Closed

The its command doesn't accept a log option (logging can't be disabled) #1450

cwohlman opened this issue Mar 12, 2018 · 14 comments · Fixed by #5519
Labels
good first issue Good for newcomers pkg/driver This is due to an issue in the packages/driver directory type: unexpected behavior User expected result, but got another

Comments

@cwohlman
Copy link

cwohlman commented Mar 12, 2018

Current behavior:

cy.window({ log: false }).its('something', { log: false }) throws an error because no options object is supported. (The docs bear this out as well).

Desired behavior:

I should be able to suppress logging using its('something', { log: false }) or the its command should inherit the log level from the previous call (if this is currently the expected behavior I'll investigate further and create a reproduction, I didn't see a mention of this in the docs).

Also, I would think that the its command should accept a timeout property.

  • Cypress Version: 2.1.0
@brian-mann
Copy link
Member

Agree. Oversight on our part.

@brian-mann brian-mann added type: unexpected behavior User expected result, but got another stage: ready for work The issue is reproducible and in scope labels Mar 12, 2018
@jennifer-shehane
Copy link
Member

We are open source and welcome contributions, even a 'work in progress' PR is helpful!

.its() and .invoke() both call into this invokeFn: https://github.com/cypress-io/cypress/blob/issue-1350/packages/driver/src/cy/commands/connectors.coffee#L124

You can see there is the log directly after it: https://github.com/cypress-io/cypress/blob/issue-1350/packages/driver/src/cy/commands/connectors.coffee#L138

Here is an example of accepting the log option and using it to log or not, for reference: https://github.com/cypress-io/cypress/blob/issue-1350/packages/driver/src/cy/commands/querying.coffee#L30

We would also need some tests around this new behavior for its and invoke

@jennifer-shehane jennifer-shehane added good first issue Good for newcomers pkg/driver This is due to an issue in the packages/driver directory labels Mar 13, 2018
@cwohlman
Copy link
Author

How do we deal with the arguments spread operator for invoke?

@bahmutov
Copy link
Contributor

Also needs timeout option, just like other commands

@wildaces215
Copy link

Is this open?

@wildaces215
Copy link

I would like handle it!

@jennifer-shehane
Copy link
Member

@wildaces215 There has been no work done on this issue. Feel free to open a pull request.

@simonhaenisch
Copy link

simonhaenisch commented May 31, 2018

I also need this, but don't have any coffeescript experience :|

Is it that invokeFn = (subject, fn, args...) -> needs to be changed to

invokeFn = (subject, fn, options = { log: true }, args...) ->

and then replace options._log = Cypress.log with

if options.log
  options._log = Cypress.log

?

Or do you rather use

_.defaults(options, {
  log: true
})

instead of function argument defaults?

Update: I actually don't really need this... in my custom command, instead of cy.request().its('body').then(body => ...) I just do cy.request().then(res => res.body ...).

Tried installing cypress on my machine to work on this, but the npm install failed with EINTEGRITY in the static package (maybe because I'm running node v8.11.2 not v.8.2.1).

@gabbersepp
Copy link
Contributor

I will take this one

@cypress-bot cypress-bot bot added stage: work in progress and removed stage: ready for work The issue is reproducible and in scope labels Oct 29, 2019
@gabbersepp
Copy link
Contributor

gabbersepp commented Oct 29, 2019

At a first step I added the Loggable options to its(). I will try a variant for invoke() in the next days
//Edit: added an option argument but had to add this as first parameter to decide if it is of type Loggable or belong to the arguments of the function call

@gabbersepp
Copy link
Contributor

I will have a look at the timeout requirement now

@gabbersepp
Copy link
Contributor

What would be the case for a timeout here? Is it a common use case that an existing object is extended with properties or functions after some time?

@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review stage: work in progress and removed stage: work in progress stage: needs review The PR code is done & tested, needs review labels Nov 6, 2019
@cypress-bot cypress-bot bot added stage: work in progress stage: needs review The PR code is done & tested, needs review and removed stage: needs review The PR code is done & tested, needs review stage: work in progress labels Nov 26, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Dec 4, 2019

The code for this is done in cypress-io/cypress#5519, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Dec 4, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Dec 12, 2019

Released in 3.8.0.

@cypress-io cypress-io locked as resolved and limited conversation to collaborators Dec 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers pkg/driver This is due to an issue in the packages/driver directory type: unexpected behavior User expected result, but got another
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants