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

normalize icons and slashes across OSes #6310

Merged
merged 5 commits into from
May 27, 2018
Merged

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented May 27, 2018

Summary

This PR uses slash in order for filepaths in test headings and console traces in output to be consistent across OSes.
It also normalizes the icons in the integration tests.

Related to #3121

Test plan

Green CI even though we're activating 30 new integration tests on windows

scripts/SkipOnWindows.js Outdated Show resolved Hide resolved
@SimenB SimenB force-pushed the windows branch 4 times, most recently from 92c8f6b to 021151e Compare May 27, 2018 13:47
@SimenB SimenB changed the title normalize icons across OSes in integration tests normalize icons and slashes across OSes May 27, 2018
@SimenB
Copy link
Member Author

SimenB commented May 27, 2018

Note that this changes output on windows machines (for the better, IMO).

Old:
image

New:
image

@codecov-io
Copy link

codecov-io commented May 27, 2018

Codecov Report

Merging #6310 into master will decrease coverage by <.01%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6310      +/-   ##
==========================================
- Coverage   63.65%   63.64%   -0.01%     
==========================================
  Files         227      227              
  Lines        8637     8638       +1     
  Branches        3        3              
==========================================
  Hits         5498     5498              
- Misses       3138     3139       +1     
  Partials        1        1
Impacted Files Coverage Δ
packages/jest-util/src/get_console_output.js 0% <0%> (ø) ⬆️
packages/jest-cli/src/reporters/utils.js 90.29% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d59017d...e6e4e6e. Read the comment docs.

@SimenB
Copy link
Member Author

SimenB commented May 27, 2018

@cpojer thoughts?

@cpojer
Copy link
Member

cpojer commented May 27, 2018 via email

@@ -76,7 +76,7 @@ export const formatTestPath = (
testPath: Path,
) => {
const {dirname, basename} = relativePath(config, testPath);
return chalk.dim(dirname + path.sep) + chalk.bold(basename);
return slash(chalk.dim(dirname + path.sep) + chalk.bold(basename));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Anybody with a Windows computer can confirm you can Ctrl+Click to get to test location? If it actually ever worked earlier of course

Copy link
Member Author

@SimenB SimenB May 27, 2018

Choose a reason for hiding this comment

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

didn't work from cmd at least (before or after). I didn't test in e.g. git bash or commandr (or what it's called, it's been years since I developed on windows)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this should work with VSCode and terminals like git bash already use such paths. microsoft/vscode#27082

Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Let's do it!

@thymikee
Copy link
Collaborator

Mind adding a changelog entry?

@SimenB
Copy link
Member Author

SimenB commented May 27, 2018

Mind adding a changelog entry?

😅 been too much hacking lately, I've forgotten all about that

@SimenB
Copy link
Member Author

SimenB commented May 27, 2018

#yolo

@SimenB SimenB merged commit d5dd58c into jestjs:master May 27, 2018
@SimenB SimenB deleted the windows branch May 27, 2018 20:39
thymikee added a commit to thymikee/jest that referenced this pull request May 27, 2018
* upstream/master:
  normalize icons and slashes across OSes (jestjs#6310)
  fix: toMatchObject throws TypeError when a source property is null (jestjs#6313)
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants