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

test: add tests for react-query package #1375

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

saul-atomrigs
Copy link
Contributor

Overview

Issue #1292

For now, I managed to add tests for postinstall.ts and part of package.ts

AS-IS

Captura de pantalla 2024-11-25 a las 6 47 22 p  m

TO-BE

Captura de pantalla 2024-11-25 a las 6 48 32 p  m

PR Checklist

  • [✅] I did below actions if need
  1. I read the Contributing Guide
  2. I added documents and tests.

Copy link

changeset-bot bot commented Nov 25, 2024

⚠️ No Changeset found

Latest commit: cba5ef5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

coauthors bot commented Nov 25, 2024

People can be co-author:

Candidate Reasons Count Add this as commit message
@manudeli #1375 (comment) #1375 (comment) #1375 (review) #1375 (review) 4 Co-authored-by: manudeli <[email protected]>
@saul-atomrigs #1375 (comment) #1375 2 Co-authored-by: saul-atomrigs <[email protected]>
@codecov-commenter #1375 (comment) 1 Co-authored-by: codecov-commenter <[email protected]>
@gwansikk #1375 (comment) 1 Co-authored-by: gwansikk <[email protected]>

Copy link

vercel bot commented Nov 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
suspensive.org ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 26, 2024 1:19am
v1.suspensive.org ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 26, 2024 1:19am
visualization.suspensive.org ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 26, 2024 1:19am

Copy link

vercel bot commented Nov 25, 2024

@saul-atomrigs is attempting to deploy a commit to the Toss Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

codspeed-hq bot commented Nov 25, 2024

CodSpeed Performance Report

Merging #1375 will create unknown performance changes

Comparing saul-atomrigs:test/react-query (cba5ef5) with main (41c7e66)

Summary

⚠️ No benchmarks were detected in both the base of the PR and the PR.\

@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.86%. Comparing base (41c7e66) to head (cba5ef5).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1375      +/-   ##
==========================================
+ Coverage   72.00%   73.86%   +1.85%     
==========================================
  Files          69       69              
  Lines         593      593              
  Branches      134      134              
==========================================
+ Hits          427      438      +11     
+ Misses        153      144       -9     
+ Partials       13       11       -2     
Components Coverage Δ
@suspensive/react 100.00% <ø> (ø)
@suspensive/react-dom 95.55% <ø> (ø)
@suspensive/react-native 100.00% <ø> (ø)
@suspensive/react-query 82.67% <ø> (+8.66%) ⬆️
@suspensive/react-query-4 0.00% <ø> (ø)
@suspensive/react-query-5 0.00% <ø> (ø)
@suspensive/jotai 0.00% <ø> (ø)
@suspensive/codemods 41.97% <ø> (ø)

Copy link
Member

@manudeli manudeli left a comment

Choose a reason for hiding this comment

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

Thanks for helping me increase my test coverage! I've left a few reviews

Comment on lines 9 to 11
const mockConsoleWarn = vi.spyOn(console, 'warn')
const mockGetPackageJson = vi.mocked(getTanStackReactQueryPackageJson)
const mockSwitchVersion = vi.mocked(switchVersion)
Copy link
Member

Choose a reason for hiding this comment

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

Long but easy to understand is better in my opinion! because we use too many package.json to name it as mockGetPackageJson

Suggested change
const mockConsoleWarn = vi.spyOn(console, 'warn')
const mockGetPackageJson = vi.mocked(getTanStackReactQueryPackageJson)
const mockSwitchVersion = vi.mocked(switchVersion)
const mockConsoleWarn = vi.spyOn(console, 'warn')
const mockGetTanStackReactQueryPackageJson = vi.mocked(getTanStackReactQueryPackageJson)
const mockSwitchVersion = vi.mocked(switchVersion)

vi.clearAllMocks()
})

it('should switch to version 4 when TanStack Query v4 is installed', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

In my opinion, It's more easy to understand. Could you reflect this review in your pull request please if you okay?

Suggested change
it('should switch to version 4 when TanStack Query v4 is installed', async () => {
it('should switch to @suspensive/react-query-4 when @tanstack/react-query@^4 is installed', async () => {

@saul-atomrigs
Copy link
Contributor Author

I applied the changes, thank you for your feedback, @manudeli !

@manudeli manudeli self-requested a review November 26, 2024 01:17
Copy link
Member

@manudeli manudeli left a comment

Choose a reason for hiding this comment

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

@saul-atomrigs Thanks! Let's improve coverage more 👍

@manudeli manudeli merged commit 6662e3f into toss:main Nov 26, 2024
16 checks passed
@gwansikk
Copy link
Collaborator

@saul-atomrigs Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants