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

chore(test): skip console messages containing 'deprecated' #1709

Merged

Conversation

matthewmayer
Copy link
Contributor

running pnpm run test logs an enormous number of deprecation warnings. This makes it hard to review the output locally and in CI.
vitest-dev/vitest#1700 shows a way to filter out specific logs, in this case anything containing "deprecated".

@matthewmayer matthewmayer requested a review from a team as a code owner January 2, 2023 15:29
@codecov
Copy link

codecov bot commented Jan 2, 2023

Codecov Report

Merging #1709 (f54e4ec) into next (7e06b47) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #1709   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files        2249     2249           
  Lines      240573   240573           
  Branches     1095     1095           
=======================================
  Hits       239698   239698           
  Misses        854      854           
  Partials       21       21           

@ST-DDT
Copy link
Member

ST-DDT commented Jan 2, 2023

Please fix the lint errors.

import-brain
import-brain previously approved these changes Jan 2, 2023
ST-DDT
ST-DDT previously approved these changes Jan 2, 2023
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

This seems to improve the execution speed in CI (3.5mins -> 2.5mins), but it doesn't have that much of an impact for me locally.

The logs have been reduced from almost 30k logs to ~200 which is good, but I'm not sure this a good long term solution.

Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Tested before and after my changes
Both still don't really work better on my machine
The tests are absolutely bloated and run into timeouts

vite.config.ts Outdated Show resolved Hide resolved
Co-authored-by: Shinigami <chrissi92@hotmail.de>
@matthewmayer matthewmayer dismissed stale reviews from ST-DDT and import-brain via dbb5b47 January 3, 2023 04:06
@matthewmayer
Copy link
Contributor Author

I don't know enough about how the tests works to figure out why they are so slow, however this change at least makes it possible to read the log output on GitHub.com easily.

@Shinigami92
Copy link
Member

[...], however this change at least makes it possible to read the log output on GitHub.com easily.

This is indeed a huge benefit 👌

@ST-DDT ST-DDT added the s: accepted Accepted feature / Confirmed bug label Jan 3, 2023
@ST-DDT ST-DDT changed the title chore(test): dont log to console messages containing 'deprecated' chore(test): skip console messages containing 'deprecated' Jan 5, 2023
@ST-DDT ST-DDT enabled auto-merge (squash) January 5, 2023 18:56
@ST-DDT ST-DDT merged commit 2a06b6a into faker-js:next Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: test p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants