[SDK] Consolidate more tests - reduce contract deployment #5258

kien-ngo · 2024-11-01T05:52:47Z

Problem solved

Short description of the bug fixed or feature added

PR-Codex overview

This PR primarily focuses on updating tests related to the ERC721 and ERC1155 contracts, including renaming and restructuring tests, adding new assertions, and removing obsolete tests.

Detailed summary

Deleted tests for mintTo and deployERC721 functionality.
Updated contract names in tests from "Test DropERC1155" to "EditionDrop" and "Test TokenERC1155" to "Edition".
Added assertions to check deployed contract names.
Introduced new tests for minting NFTs with different input types.
Removed reliance on secret keys in certain tests.

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

vercel · 2024-11-01T05:52:50Z

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	Nov 1, 2024 6:56pm
thirdweb_playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 1, 2024 6:56pm
thirdweb-www	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 1, 2024 6:56pm
wallet-ui	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 1, 2024 6:56pm

changeset-bot · 2024-11-01T05:52:50Z

⚠️ No Changeset found

Latest commit: 7a86344

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

graphite-app · 2024-11-01T05:52:51Z

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

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

kien-ngo · 2024-11-01T05:53:00Z

[SDK] Consolidate more tests - reduce contract deployment #5258 👈
main

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

Join @kien-ngo and the rest of your teammates on Graphite

github-actions · 2024-11-01T05:56:02Z

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
thirdweb (esm)	43 KB (0%)	861 ms (0%)	3.3 s (-3.44% 🔽)	4.1 s
thirdweb (cjs)	102.91 KB (0%)	2.1 s (0%)	5.8 s (-22.72% 🔽)	7.8 s
thirdweb (minimal + tree-shaking)	4.84 KB (0%)	97 ms (0%)	164 ms (+27.21% 🔺)	261 ms
thirdweb/chains (tree-shaking)	498 B (0%)	10 ms (0%)	57 ms (-69.4% 🔽)	67 ms
thirdweb/react (minimal + tree-shaking)	16.98 KB (0%)	340 ms (0%)	667 ms (-0.9% 🔽)	1.1 s

codecov · 2024-11-01T07:15:30Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.37%. Comparing base (c621c13) to head (7a86344).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5258   +/-   ##
=======================================
  Coverage   45.37%   45.37%           
=======================================
  Files        1067     1067           
  Lines       55450    55450           
  Branches     3991     3989    -2     
=======================================
  Hits        25161    25161           
  Misses      29598    29598           
  Partials      691      691

Flag	Coverage Δ		*Carryforward flag
legacy_packages	`65.68% <ø> (ø)`		Carriedforward from c621c13
packages	`40.41% <ø> (ø)`

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

see 2 files with indirect coverage changes

joaquim-verges · 2024-11-01T08:28:33Z

packages/thirdweb/src/extensions/erc1155/drop1155.test.ts

        params: {
-          name: "Test DropERC1155",
-          contractURI: TEST_CONTRACT_URI,
+          name: "EditionDrop",


Can You still pass a contract uri here? Was useful to skip the ipsy upload

joaquim-verges · 2024-11-01T08:29:17Z

packages/thirdweb/src/extensions/erc1155/token1155.test.ts

      params: {
-        name: "Test TokenERC1155",
-        contractURI: TEST_CONTRACT_URI,
+        name: "Edition",


Same here. Also this tests the exact same thing no? Doesn't actually help coverage

joaquim-verges · 2024-11-01T08:33:01Z

packages/thirdweb/src/extensions/erc721/token721.test.ts

+    expect(isGetNFTSupported(selectors)).toBe(true);
+  });
+
+  it("should return a default value if the URI of the token doesn't exist", async () => {


What's the value of moving these tests around? Feels like unnecessary work. Would you rather focus on actually writing new tests on areas that aren't covered.

You can check the uncovered areas on codecov, any file/folder with low coverage is what you should focus on.

All of react for example is particularly low

packages/thirdweb/src/extensions/erc721/token721.test.ts

gregfromstl · 2024-11-01T18:08:46Z

Merge activity

Nov 1, 2:08 PM EDT: The merge label 'merge-queue' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
Nov 1, 2:09 PM EDT: A user added this pull request to the Graphite merge queue.
Nov 1, 2:56 PM EDT: A user merged this pull request with the Graphite merge queue.

## Problem solved Short description of the bug fixed or feature added  --- ## PR-Codex overview This PR focuses on cleaning up and enhancing tests related to `ERC721` and `ERC1155` contracts in the `thirdweb` package. It includes the removal of obsolete tests and the addition of new functionality checks, particularly around contract deployment and NFT minting. ### Detailed summary - Deleted obsolete test files for `ERC721` and `ERC20`. - Updated `mintAdditionalSupplyToBatch.test.ts` to include checks for contract names. - Enhanced tests for `DropERC1155` and `TokenERC1155` to verify deployed names. - Removed tests for `deployERC721` that were reliant on secret keys. - Added new tests for minting NFTs in `token721.test.ts`, covering both object and string inputs. - Ensured compatibility checks for NFT functions in the new tests. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}`

github-actions bot added the packages label Nov 1, 2024

kien-ngo changed the title ~~update~~ [SDK] Consolidate more tests - reduce contract deployment Nov 1, 2024

kien-ngo marked this pull request as ready for review November 1, 2024 05:53

kien-ngo requested review from gregfromstl, jnsdls and joaquim-verges as code owners November 1, 2024 05:53

vercel bot deployed to Preview – wallet-ui November 1, 2024 05:55 View deployment

vercel bot deployed to Preview – thirdweb_playground November 1, 2024 05:55 View deployment

vercel bot deployed to Preview – thirdweb-www November 1, 2024 06:03 View deployment

vercel bot deployed to Preview – docs-v2 November 1, 2024 06:05 View deployment

kien-ngo force-pushed the kien/remove-deploy-tests branch from ebc33b2 to 681c450 Compare November 1, 2024 06:08

vercel bot deployed to Preview – wallet-ui November 1, 2024 06:09 View deployment

vercel bot deployed to Preview – thirdweb_playground November 1, 2024 06:09 View deployment

vercel bot deployed to Preview – docs-v2 November 1, 2024 06:11 View deployment

vercel bot deployed to Preview – thirdweb-www November 1, 2024 06:11 View deployment

kien-ngo force-pushed the kien/remove-deploy-tests branch from 681c450 to 998fc1f Compare November 1, 2024 07:09

vercel bot deployed to Preview – wallet-ui November 1, 2024 07:11 View deployment

vercel bot deployed to Preview – thirdweb_playground November 1, 2024 07:12 View deployment

vercel bot deployed to Preview – docs-v2 November 1, 2024 07:20 View deployment

vercel bot deployed to Preview – thirdweb-www November 1, 2024 07:20 View deployment

joaquim-verges reviewed Nov 1, 2024

View reviewed changes

kien-ngo force-pushed the kien/remove-deploy-tests branch from 998fc1f to ca25e6c Compare November 1, 2024 10:44

graphite-app bot reviewed Nov 1, 2024

View reviewed changes

packages/thirdweb/src/extensions/erc721/token721.test.ts Outdated Show resolved Hide resolved

vercel bot deployed to Preview – wallet-ui November 1, 2024 10:46 View deployment

vercel bot deployed to Preview – thirdweb_playground November 1, 2024 10:47 View deployment

kien-ngo force-pushed the kien/remove-deploy-tests branch 2 times, most recently from 7658b51 to f29dcaa Compare November 1, 2024 10:50

vercel bot deployed to Preview – wallet-ui November 1, 2024 10:52 View deployment

vercel bot deployed to Preview – thirdweb_playground November 1, 2024 10:54 View deployment

kien-ngo requested a review from joaquim-verges November 1, 2024 10:57

vercel bot deployed to Preview – thirdweb-www November 1, 2024 11:03 View deployment

vercel bot deployed to Preview – docs-v2 November 1, 2024 11:04 View deployment

gregfromstl approved these changes Nov 1, 2024

View reviewed changes

gregfromstl added the merge-queue Adds the pull request to Graphite's merge queue. label Nov 1, 2024

gregfromstl force-pushed the kien/remove-deploy-tests branch from f29dcaa to 7a86344 Compare November 1, 2024 18:37

vercel bot deployed to Preview – wallet-ui November 1, 2024 18:46 View deployment

vercel bot deployed to Preview – thirdweb_playground November 1, 2024 18:47 View deployment

vercel bot deployed to Preview – docs-v2 November 1, 2024 18:54 View deployment

vercel bot deployed to Preview – thirdweb-www November 1, 2024 18:56 View deployment

graphite-app bot merged commit 7a86344 into main Nov 1, 2024
31 checks passed

graphite-app bot deleted the kien/remove-deploy-tests branch November 1, 2024 18:56

vercel bot deployed to Production – wallet-ui November 1, 2024 18:58 View deployment

vercel bot deployed to Production – thirdweb_playground November 1, 2024 18:58 View deployment

vercel bot deployed to Production – docs-v2 November 1, 2024 19:00 View deployment

vercel bot deployed to Production – thirdweb-www November 1, 2024 19:06 View deployment

[SDK] Consolidate more tests - reduce contract deployment #5258

[SDK] Consolidate more tests - reduce contract deployment #5258

Conversation

kien-ngo commented Nov 1, 2024 • edited by pr-codex bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem solved

PR-Codex overview

Detailed summary

Uh oh!

vercel bot commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

graphite-app bot commented Nov 1, 2024

Your org has enabled the Graphite merge queue for merging into main

Uh oh!

kien-ngo commented Nov 1, 2024

Uh oh!

github-actions bot commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

codecov bot commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joaquim-verges Nov 1, 2024

Choose a reason for hiding this comment

Uh oh!

kien-ngo Nov 1, 2024

Choose a reason for hiding this comment

Uh oh!

kien-ngo Nov 1, 2024

Choose a reason for hiding this comment

Uh oh!

joaquim-verges Nov 1, 2024

Choose a reason for hiding this comment

Uh oh!

kien-ngo Nov 1, 2024

Choose a reason for hiding this comment

Uh oh!

joaquim-verges Nov 1, 2024

Choose a reason for hiding this comment

Uh oh!

kien-ngo Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gregfromstl commented Nov 1, 2024 • edited by graphite-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kien-ngo commented Nov 1, 2024 •

edited by pr-codex bot

Loading

vercel bot commented Nov 1, 2024 •

edited

Loading

changeset-bot bot commented Nov 1, 2024 •

edited

Loading

github-actions bot commented Nov 1, 2024 •

edited

Loading

codecov bot commented Nov 1, 2024 •

edited

Loading

kien-ngo Nov 1, 2024 •

edited

Loading

gregfromstl commented Nov 1, 2024 •

edited by graphite-app bot

Loading