feat(Unification): add internal log group api and consume in .within() #20474
Conversation
|
Thanks for taking the time to open a PR!
|
emilyrohrbough
changed the title
|
|
||
| const subject = userOptions.$el || null | ||
|
|
||
| const log = Cypress.log(options) |
There was a problem hiding this comment.
When Cypress.log has emitOnly: true it will not send emit the log to the runner/reporter and will not return a log instance. It will however, create the log attributes and store it in the LogManager and add the logs on the Command. This seems to work just fine, however,
- Is there risk of data leaking when this test/command is sent to the dashbaord? I didn't find this attribute in the schema, but wanted to double check.
- Is there risk of this behaving unexpectedly if the user were to pass
{ log: false }to the command consuming this?
For example:
cy.get('#form').within({ log: false}, () => {})
This example is likely less applicable to .within() and more applicable to up-coming consumption in .withToDomain().
Merge branch 'cmd-group-logic' of https://github.com/cypress-io/cypress into cmd-group-logic
|
I think it visually looks super odd for the darkest background color not to extend the full width of the row. It's currently "indenting" with the caret. Is this intentional? Also appears that the command log numbers themselves are indenting as well. |
|
@brian-mann That is the current designs of 10.0. The follow up PR to update the styles will remove this: #20465 |
|
I see, and the styles are expected to merge first right? |
|
It wouldn't really matter which order these went in first. I figured merging this first might be better so it can unblock multi-domain. 🤷🏻♀️ Would you prefer the other way around? |
| @@ -357,7 +357,7 @@ describe('commands', () => { | |||
| }) | |||
| }) | |||
|
|
|||
| it('group is closed by default when all nested command have passed', () => { | |||
| it('group is open by default when all nested command have passed', () => { | |||
There was a problem hiding this comment.
I love this change. No more command log peekaboo
packages/driver/src/cypress/log.ts
Outdated
| export interface LogConfig { | ||
| // defaults to command | ||
| instrument?: 'agent' | 'command' | 'route' | ||
| // name of the log | ||
| name?: string | ||
| // additional information to include in the log if not overridden | ||
| // the render props message | ||
| // defaults to command arguments for command instrument | ||
| message?: string | ||
| // the JQuery element for the command. This will highlight the command | ||
| // in the main window when debugging | ||
| $el?: JQuery | ||
| // whether or not to show the log in the Reporter UI or only | ||
| // store the log details on the command and log manager | ||
| emitOnly?: boolean | ||
| // whether or not to start a new log group | ||
| groupStart?: boolean | ||
| // timeout of the group command - defaults to defaultCommandTimeout | ||
| timeout?: number | ||
| } |
There was a problem hiding this comment.
I think this should be combined somehow with the LogConfig defined in internal-types.d.ts:
cypress/packages/driver/types/internal-types.d.ts
Lines 51 to 70 in 938f72a
packages/driver/src/cy/logGroup.ts
Outdated
| import $errUtils from '../cypress/error_utils' | ||
| import type { LogConfig } from '../cypress/log' | ||
|
|
||
| export interface LogGroupConfig { |
There was a problem hiding this comment.
doesn't this extend LogConfig since we pass it to Cypress.log?
There was a problem hiding this comment.
@flotwig I purposely limited these options to keep this API clean and minimal. As additional use-cases are determined, we can expand this API.
|
Moving to draft. This will go in after the command styles are merged. |
|
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Summary
.within()to consume log group apiUser facing changelog
Enhanced
.within()to provide visual indication of nested commands and logs.How has the user experience changed?
PR Tasks
cypress-documentation?type definitions?cypress.schema.json?