Skip to content

[SDK] Test coverage TOOL-2241 #5826

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

Merged
merged 1 commit into from
Dec 28, 2024
Merged

Conversation

kien-ngo
Copy link
Contributor

@kien-ngo kien-ngo commented Dec 22, 2024

Holiday project.
Basically trying to improve test coverage, especially in those low-haning-fruit utils files


PR-Codex overview

This PR primarily focuses on enhancing the codebase by adding internal documentation, improving error handling in various functions, and ensuring consistency in function exports. Additionally, it includes new test cases for several utility functions.

Detailed summary

  • Added @internal annotations to Queue and convertViemChain.
  • Changed function definitions to use export for better visibility.
  • Updated test cases to use await expect(...) for promise handling.
  • Fixed typos in import paths.
  • Added new tests for namehash, encodeLabelhash, and other utility functions.
  • Improved error handling in parseNftUri and getContract.
  • Enhanced utility functions like isHttpUrl, formatUniversalUrl, and formatNativeUrl with more test coverage.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Copy link

changeset-bot bot commented Dec 22, 2024

⚠️ No Changeset found

Latest commit: 03bf859

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

vercel bot commented Dec 22, 2024

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

Name Status Preview Comments Updated (UTC)
docs-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 28, 2024 0:25am
thirdweb_playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 28, 2024 0:25am
thirdweb-www ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 28, 2024 0:25am
wallet-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 28, 2024 0:25am

@github-actions github-actions bot added packages SDK Involves changes to the thirdweb SDK labels Dec 22, 2024
Copy link
Contributor Author


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • merge-queue - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@kien-ngo kien-ngo changed the title update [SDK] React test coverage Dec 22, 2024
@kien-ngo kien-ngo changed the title [SDK] React test coverage [SDK] React test coverage Dec 22, 2024
@kien-ngo kien-ngo changed the title [SDK] React test coverage [SDK] React test coverage TOOL-2241 Dec 22, 2024
Copy link

linear bot commented Dec 22, 2024

@kien-ngo kien-ngo marked this pull request as ready for review December 22, 2024 13:42
@kien-ngo kien-ngo requested review from a team as code owners December 22, 2024 13:42
@kien-ngo kien-ngo added the DO NOT MERGE This pull request is still in progress and is not ready to be merged. label Dec 22, 2024 — with Graphite App
Copy link
Contributor

github-actions bot commented Dec 22, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 45.09 KB (+0.06% 🔺) 902 ms (+0.06% 🔺) 548 ms (+55.22% 🔺) 1.5 s
thirdweb (cjs) 110.84 KB (-0.09% 🔽) 2.3 s (-0.09% 🔽) 668 ms (-19.31% 🔽) 2.9 s
thirdweb (minimal + tree-shaking) 5.58 KB (0%) 112 ms (0%) 278 ms (+317.13% 🔺) 390 ms
thirdweb/chains (tree-shaking) 506 B (0%) 10 ms (0%) 56 ms (+320.85% 🔺) 66 ms
thirdweb/react (minimal + tree-shaking) 19.12 KB (0%) 383 ms (0%) 228 ms (+207.57% 🔺) 611 ms

Copy link

codecov bot commented Dec 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.18%. Comparing base (a7b6cb3) to head (03bf859).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5826      +/-   ##
==========================================
+ Coverage   54.52%   55.18%   +0.66%     
==========================================
  Files        1105     1123      +18     
  Lines       59332    59611     +279     
  Branches     4921     5030     +109     
==========================================
+ Hits        32349    32895     +546     
+ Misses      26263    25996     -267     
  Partials      720      720              
Flag Coverage Δ *Carryforward flag
legacy_packages 65.68% <ø> (ø) Carriedforward from a7b6cb3
packages 52.83% <100.00%> (+0.82%) ⬆️

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
packages/thirdweb/src/chains/utils.ts 90.43% <100.00%> (+6.08%) ⬆️
...tecode/extractMinimalProxyImplementationAddress.ts 100.00% <ø> (ø)
...irdweb/src/utils/bytecode/resolveImplementation.ts 80.80% <ø> (ø)
packages/thirdweb/src/utils/promise/p-limit.ts 94.01% <100.00%> (+90.59%) ⬆️
packages/thirdweb/src/utils/semver.ts 100.00% <100.00%> (+36.84%) ⬆️

... and 34 files with indirect coverage changes

Copy link
Member

gregfromstl commented Dec 27, 2024

Merge activity

  • Dec 27, 6:05 PM EST: The merge label 'merge-queue' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Dec 27, 6:05 PM EST: A user added this pull request to the Graphite merge queue.
  • Dec 27, 6:09 PM EST: The Graphite merge queue couldn't merge this PR because it was not satisfying all requirements (Failed CI: 'Unit Tests').
  • Dec 27, 7:26 PM EST: A user added this pull request to the Graphite merge queue.
  • Dec 27, 7:26 PM EST: A user merged this pull request with the Graphite merge queue.

gregfromstl pushed a commit that referenced this pull request Dec 27, 2024
Holiday project.
Basically trying to improve test coverage, especially in those low-haning-fruit utils files

<!-- start pr-codex -->

---

## PR-Codex overview
This PR focuses on enhancing documentation, improving error handling, and adding tests across various modules in the codebase. It also corrects minor typos and ensures better code readability.

### Detailed summary
- Added `@internal` documentation comments to `Queue` and `convertViemChain`.
- Updated test cases to use `await expect(...)` for consistency.
- Fixed typos in file imports and function names.
- Introduced new tests for `namehash`, `encodeLabelhash`, and `parseNFT`.
- Enhanced error handling in `parseNftUri`.
- Improved the `toSemver` function and its tests.
- Added tests for `replaceBigInts` and `toHex` functions.

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

<!-- end pr-codex -->
@gregfromstl gregfromstl force-pushed the kien/react-code-coverage-2 branch from 7d7fec7 to 748edf9 Compare December 27, 2024 23:05
@graphite-app graphite-app bot removed the merge-queue Adds the pull request to Graphite's merge queue. label Dec 27, 2024
Holiday project.
Basically trying to improve test coverage, especially in those low-haning-fruit utils files

<!-- start pr-codex -->

---

## PR-Codex overview
This PR focuses on enhancing documentation, improving error handling, and adding tests across various modules in the codebase. It also corrects minor typos and ensures better code readability.

### Detailed summary
- Added `@internal` documentation comments to `Queue` and `convertViemChain`.
- Updated test cases to use `await expect(...)` for consistency.
- Fixed typos in file imports and function names.
- Introduced new tests for `namehash`, `encodeLabelhash`, and `parseNFT`.
- Enhanced error handling in `parseNftUri`.
- Improved the `toSemver` function and its tests.
- Added tests for `replaceBigInts` and `toHex` functions.

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

<!-- end pr-codex -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages SDK Involves changes to the thirdweb SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants