Test/diamond cut facet by megabyte0x · Pull Request #151 · Perfect-Abstractions/Compose

megabyte0x · 2025-10-30T10:50:39Z

Tests for DiamondCutFacet, LibDiamond and DiamondLoupeFacet.

Issue: #119

mudgen · 2025-10-30T20:59:01Z

@megabyte0x This pull request is very much appreciated.

One thing that is sorely need, (very very much) needed right now is tests that show gas usage for very large diamonds, for example 200 facets with 10 functions each, added to a diamond. This is especially needed for testing and seeing the gas used by the functions in DiamondLoupeFacet for the diamond gas challenge.

Is this something that can be added fairly easily to these tests?

github-actions · 2025-10-31T01:24:18Z

Coverage Report

Metric	Coverage	Details
Lines	58%	819/1415 lines
Functions	66%	160/244 functions
Branches	40%	100/251 branches

Last updated: Mon, 03 Nov 2025 21:10:38 GMT for commit fa1cf3e

github-actions · 2025-10-31T01:24:33Z

Gas Report

No gas usage changes detected between main and test/DiamondCutFacet.

All functions maintain the same gas costs. ✅

Last updated: Mon, 03 Nov 2025 21:11:02 GMT for commit fa1cf3e

megabyte0x · 2025-10-31T10:14:03Z

Is this something that can be added fairly easily to these tests?

hey @mudgen
I am not sure about the difficulty level, but I am almost done with DiamondCutFacet tests. Will keep this in consideration while testing for the LoupeFacet.

megabyte0x · 2025-10-31T12:26:53Z

hey @mudgen

I am done with the DiamondCutFacet.sol tests with 100% coverage.

This coverage was only achievable when I removed the following IncorrectFacetCutAction branch as its unreachable. I also mentioned the same in the discussion here.

mudgen · 2025-10-31T17:16:52Z

@adamgall Do the tests look okay to you?

lumoswiz · 2025-11-02T21:09:32Z

@megabyte0x This pull request is very much appreciated.

One thing that is sorely need, (very very much) needed right now is tests that show gas usage for very large diamonds, for example 200 facets with 10 functions each, added to a diamond. This is especially needed for testing and seeing the gas used by the functions in DiamondLoupeFacet for the diamond gas challenge.

Is this something that can be added fairly easily to these tests?

See #165

adamgall · 2025-11-02T21:22:51Z

Let's remove this file from the PR for now, please

adamgall · 2025-11-02T21:26:33Z

+    function test_DiamondCut_initalizeCallWithWrongCalldataReturningErrorMessage() public {
+        (DiamondCutFacet.FacetCut[] memory _cut,,) = _basicAction();
+
+        ERC20FacetHarnessWithFallback newFacet = new ERC20FacetHarnessWithFallback();
+        address _init = address(newFacet);
+
+        bytes memory _wrongCalldata = abi.encodeWithSelector(
+            bytes4(keccak256("initialize(string,string,uint256)")), TOKEN_NAME, TOKEN_DECIMALS, TOKEN_DECIMALS
+        );
+
+        vm.prank(owner);
+        vm.expectRevert(abi.encode("WRONG FUNCTION CALL"));
+        facet.diamondCut(_cut, _init, _wrongCalldata);
+    }


I'm probably missing something, but can you explain what this test is testing for?

The revert comes from the ERC20FacetHarnessWithFallbackHarness (not an actual src contract), so it seems to me like this test doesn't actually hit a real contract.

This test is for the following condition branch when the _init address sends an error message on failed execution of the _calldata.

ERC20FacetHarnessWithFallbackHarness inherits the ERC20Facet which is being as the _init contract for testing, like in other test cases ERC20Harness is utilized.

megabyte0x · 2025-11-03T11:43:33Z

@mudgen LibDiamond.sol is also 100% covered. Let me know if you think I should start with DiamondLoupe.sol test because its being updated right now for gas optimization.

mudgen · 2025-11-03T14:14:37Z

@megabyte0x Great! Love to see this.

@adamgall is this ready for us to merge?

@megabyte0x You can work on DiamondLoupeFacet testing but some things for you to know are that some gas tests for it were merged in recently. Also, the DiamondLoupeFacet function implementations are changing, so there will probably be more aspects/conditions/code to test after the new implementations are merged in.

mudgen · 2025-11-03T14:15:43Z

@megabyte0x Format check failed. Need to run forge fmt, preferably with forge version 1.4.3-stable

megabyte0x · 2025-11-03T21:05:02Z

@megabyte0x Format check failed. Need to run forge fmt, preferably with forge version 1.4.3-stable

fixed this.

@mudgen If this PR is ready for merge as per you, then let's merge this. I'll work on the DiamondLoupeFacet when its ready. Till then I'll contribute to other issues.

mudgen · 2025-11-03T21:11:44Z

@megabyte0x Great, thanks very much. Merged.

…ondCutFacet Test/diamond cut facet

megabyte0x added 5 commits October 27, 2025 19:05

feat: DiamondCutFacetHarness

62dcb72

feat: add, remove and replace action tests

d1c61ef

feat: Multiple actions test

c9f2425

feat: initialize call and empty function selector array test

8f04b1a

feat: initalize call with wrong calldata test

b1a3194

megabyte0x added 2 commits October 31, 2025 16:19

feat: tests for immutable functions

ec609b7

100% test coverage for DiamondCutFacet

e6513e6

mudgen requested a review from adamgall October 31, 2025 17:16

init: LibDiamond Tests

56ea27a

adamgall suggested changes Nov 2, 2025

View reviewed changes

megabyte0x added 4 commits November 3, 2025 15:05

fix: remove coverage.txt and rename the harness file

0861949

feat: error condition tests

58a0d1f

feat: more error conditions tests

6802161

LibDiamond.sol 100% coverage

8d2f23a

megabyte0x and others added 2 commits November 4, 2025 02:30

Merge branch 'Perfect-Abstractions:main' into test/DiamondCutFacet

eb9c156

fix: fmt

fa1cf3e

mudgen marked this pull request as ready for review November 3, 2025 21:11

mudgen merged commit 2f3b3b2 into Perfect-Abstractions:main Nov 3, 2025
4 checks passed

megabyte0x deleted the test/DiamondCutFacet branch November 4, 2025 08:34

JackieXu pushed a commit to JackieXu/Compose that referenced this pull request Nov 6, 2025

Merge pull request Perfect-Abstractions#151 from megabyte0x/test/Diam…

1af0842

…ondCutFacet Test/diamond cut facet

Conversation

megabyte0x commented Oct 30, 2025

Tests for DiamondCutFacet, LibDiamond and DiamondLoupeFacet.

Uh oh!

mudgen commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Uh oh!

github-actions Bot commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Gas Report

Uh oh!

megabyte0x commented Oct 31, 2025

Uh oh!

megabyte0x commented Oct 31, 2025

Uh oh!

mudgen commented Oct 31, 2025

Uh oh!

lumoswiz commented Nov 2, 2025

Uh oh!

adamgall Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

megabyte0x Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

adamgall Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

megabyte0x Nov 3, 2025

Choose a reason for hiding this comment

Uh oh!

megabyte0x commented Nov 3, 2025

Uh oh!

mudgen commented Nov 3, 2025

Uh oh!

mudgen commented Nov 3, 2025

Uh oh!

megabyte0x commented Nov 3, 2025

Uh oh!

mudgen commented Nov 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mudgen commented Oct 30, 2025 •

edited

Loading

github-actions Bot commented Oct 31, 2025 •

edited

Loading

github-actions Bot commented Oct 31, 2025 •

edited

Loading