Skip to content

Clean up console.log tests#28693

Merged
rickhanlonii merged 2 commits into
react:mainfrom
rickhanlonii:rh/test-cleanup
Apr 1, 2024
Merged

Clean up console.log tests#28693
rickhanlonii merged 2 commits into
react:mainfrom
rickhanlonii:rh/test-cleanup

Conversation

@rickhanlonii

Copy link
Copy Markdown
Contributor

Followups to #28680

One of these test don't need to use console.log.

The others are specifically testing console.log behavior, so I added a comment.

@react-sizebot

react-sizebot commented Mar 31, 2024

Copy link
Copy Markdown

Comparing: df95577...42b4189

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 177.10 kB 177.10 kB = 55.05 kB 55.05 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 172.94 kB 172.94 kB = 53.91 kB 53.91 kB
facebook-www/ReactDOM-prod.classic.js = 592.68 kB 592.68 kB = 103.96 kB 103.96 kB
facebook-www/ReactDOM-prod.modern.js = 574.37 kB 574.37 kB = 100.98 kB 100.98 kB
test_utils/ReactAllWarnings.js Deleted 64.93 kB 0.00 kB Deleted 16.24 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js Deleted 64.93 kB 0.00 kB Deleted 16.24 kB 0.00 kB

Generated by 🚫 dangerJS against 42b4189

@yungsters yungsters left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great! With 1 minor change needed.

Comment thread packages/react-client/src/__tests__/ReactFlight-test.js
@rickhanlonii rickhanlonii merged commit e9ae2c8 into react:main Apr 1, 2024
@rickhanlonii rickhanlonii deleted the rh/test-cleanup branch April 1, 2024 14:50
rickhanlonii added a commit that referenced this pull request Apr 1, 2024
Followup from #28693 and
#28680.

In CI, we fail the test for any unasserted console.log. In DEV, we don't
fail, but you can still use the matchers and we'll assert on them.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Followups to react#28680

One of these test don't need to use `console.log`. 

The others are specifically testing `console.log` behavior, so I added a
comment.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Followup from react#28693 and
react#28680.

In CI, we fail the test for any unasserted console.log. In DEV, we don't
fail, but you can still use the matchers and we'll assert on them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants