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

Display full specfile(s) in stdout during cypress run #5120

Merged
merged 54 commits into from
Oct 21, 2019

Conversation

jennifer-shehane
Copy link
Member

@jennifer-shehane jennifer-shehane commented Sep 10, 2019

User facing changelog

Stdout printed during cypress run no longer truncates text such a filenames and instead wraps the text to fit within the width of the terminal.

Additional details

The way tables are printed in stdout has been refactored.

Paths now insert newlines
  • Previously, paths would extend beyond the width of the terminal or (even less ideal) would truncate the path with ..., now we have a new method formatPath that when passed the width of the table's column it will be drawn within, will add newlines at the width so that the path will return.

https://github.com/cypress-io/cypress/blob/issue-4977-specfiles-in-stdout/packages/server/lib/modes/run.js#L115

  • Many places where we were just doing console.log('path/to/thing') have now been replaced with tables so we can properly wrap depending on the set column width.

https://github.com/cypress-io/cypress/blob/issue-4977-specfiles-in-stdout/packages/server/lib/modes/run.js#L859

Leftover width when < 100 width
  • Before, the algorithm was naive about throwing extra leftover sizing into the width of the first column, which would be confusing if you assigned the width of the column to 3 (1 left padding, 1 width, 1 right padding), it would sometimes print out at a width of 10 because if the other columns did not take up a width of 100 - they would throw the extra width into the first column.
  • Now the algorithm respects the width assigned and subtracts the width taken up by borders from the widest column.

https://github.com/cypress-io/cypress/blob/issue-4977-specfiles-in-stdout/packages/server/lib/util/terminal.js#L161

Snapshots
  • Many of the snapshot replacements have been updated in order to not disrupt the 100 width of the original table. When replacing text from a snapshot like 1028x468, we now replace it with the smallest possible width (in this case a screenshot could be 0x0 at it's smallest) and add padding to equal the length of the original screenshot, so we replace the text like: _.startPad('YxZ', p1.length).

https://github.com/cypress-io/cypress/blob/issue-4977-specfiles-in-stdout/packages/server/test/support/helpers/e2e.coffee#L77

  • Additionally, in one area replacing the text in the snapshot was too late. The width of the tables had already been calculated and drawn (with the newline returns), so replacing the text at that point would mess up all of the table alignments. This is when the stdout is printing the current working directory. In this case, we pass a FAKE_CWD_PATH env var during test execution - that when present during run, replaces the current working directory path before we insert the newlines.

https://github.com/cypress-io/cypress/blob/issue-4977-specfiles-in-stdout/packages/server/lib/modes/run.js#L120

Bug in cli-table-3

I found a bug with cli-table-3 where padding passed to a cell is ignored if set to 0. Opened an issue here: cli-table/cli-table3#124

How has the user experience changed?

Run Header

Before

Screen Shot 2019-10-17 at 9 47 23 AM

After

Screen Shot 2019-10-17 at 9 38 55 AM

Run Results

Before

Screen Shot 2019-10-17 at 9 47 36 AM

After

Screen Shot 2019-10-17 at 9 38 30 AM

PR Tasks

@cypress

This comment has been minimized.

@jennifer-shehane jennifer-shehane changed the title [WIP] list full specfile in stdout Display full specfile(s) in stdout during cypress run Sep 12, 2019
Copy link
Member

@brian-mann brian-mann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • We need to leave the truncation in place for displaying the number of found spec files - because using a specPattern would list out potentially dozens or even hundreds of tests - which is not what the user wants.
  • Add margin-right space back in for displaying the Running spec

packages/server/lib/modes/run.js Outdated Show resolved Hide resolved
@brian-mann

This comment has been minimized.

@jennifer-shehane

This comment has been minimized.

@jennifer-shehane

This comment has been minimized.

@jennifer-shehane

This comment has been minimized.

# Conflicts:
#
packages/server/__snapshots__/1_commands_outside_of_test_spec.coffee.js
#	packages/server/__snapshots__/2_browser_path_spec.coffee.js
#	packages/server/__snapshots__/2_cookies_spec.coffee.js
#	packages/server/__snapshots__/3_plugins_spec.coffee.js
#	packages/server/__snapshots__/3_user_agent_spec.coffee.js
#	packages/server/__snapshots__/4_request_spec.coffee.js
#	packages/server/__snapshots__/5_screenshots_spec.coffee.js
#	packages/server/__snapshots__/5_stdout_spec.coffee.js
#	packages/server/__snapshots__/5_subdomain_spec.coffee.js
#	packages/server/__snapshots__/6_web_security_spec.coffee.js
- This was throwing off the table calculations.
…count for shorter duration printing when it is 1 second
@jennifer-shehane jennifer-shehane changed the title [WIP] Display full specfile(s) in stdout during cypress run Display full specfile(s) in stdout during cypress run Oct 17, 2019
# Conflicts:
#
packages/server/__snapshots__/1_commands_outside_of_test_spec.coffee.js
#	packages/server/__snapshots__/2_cookies_spec.coffee.js
#	packages/server/__snapshots__/3_plugins_spec.coffee.js
#	packages/server/__snapshots__/3_user_agent_spec.coffee.js
#	packages/server/__snapshots__/4_request_spec.coffee.js
#	packages/server/__snapshots__/5_subdomain_spec.coffee.js
#	packages/server/__snapshots__/5_task_not_registered_spec.coffee.js
#	packages/server/__snapshots__/6_uncaught_support_file_spec.coffee.js
#	packages/server/test/e2e/5_stdout_spec.coffee
#	packages/server/test/support/helpers/e2e.coffee
flotwig
flotwig previously approved these changes Oct 17, 2019
Copy link
Contributor

@flotwig flotwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New stdout looks awesome! I noticed some minor things you might want to fix but all in all this looks great.

packages/server/test/support/helpers/e2e.coffee Outdated Show resolved Hide resolved
packages/server/lib/util/terminal.js Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Full path to failed specfile is not viewable from stdout
4 participants