js-sdk icon indicating copy to clipboard operation
js-sdk copied to clipboard

Feature/lit 3017 auth unification migrate actual tests

Open Ansonhkg opened this issue 1 year ago • 2 comments

Description

This PR merges several other PRs https://github.com/LIT-Protocol/js-sdk/pull/439 https://github.com/LIT-Protocol/js-sdk/pull/438 https://github.com/LIT-Protocol/js-sdk/pull/436, so we can run the tests as they rely on the new features introduced in those branches.

Please check this directory for all the tests

Changes

  • Introduced a helper function for composing an access control resource string. Previously, we only required dataToHash, but it turns out that the string of the hashed access control string must be prepended before it. See 72bcd8d6485bf7a9dfe12c1a4e9e1a177841d9e4.
  • Better types on Node response https://github.com/LIT-Protocol/js-sdk/pull/440/commits/fe71ddfba4c9701c513bb649a4e53af45ea66bd8
  • Unfortunately node-fetch is needed when running against local tests for some reasons. The package is added a dev dependency so won't affect production. It's only being used in this test. 757d588dedc66864d0fcec692e9a22677ce38f0f

Also want to note that we should write scenario in the test/JsDocs for more complex operations, see example https://github.com/LIT-Protocol/js-sdk/blob/757d588dedc66864d0fcec692e9a22677ce38f0f/local-tests/tests/testDelegatingCapacityCreditsNFTToAnotherWalletToExecuteJs.ts#L6-L19

Type of change

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] This change requires a documentation update

How Has This Been Tested?

 NETWORK=localchain DEBUG=true yarn test:local         
image

Checklist:

  • [x] My code follows the style guidelines of this project
  • [x] I have performed a self-review of my code
  • [x] I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] My changes generate no new warnings
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • [ ] New and existing unit tests pass locally with my changes
  • [ ] Any dependent changes have been merged and published in downstream modules

Ansonhkg avatar Apr 24 '24 15:04 Ansonhkg

This PR merges several other PRs https://github.com/LIT-Protocol/js-sdk/pull/439 https://github.com/LIT-Protocol/js-sdk/pull/438 https://github.com/LIT-Protocol/js-sdk/pull/436, so we can run the tests as they rely on the new features introduced in those branches.

Is it juts a merged PR? So what's to review here? @Ansonhkg

DashKash54 avatar Apr 25 '24 00:04 DashKash54

This PR merges several other PRs https://github.com/LIT-Protocol/js-sdk/pull/439 https://github.com/LIT-Protocol/js-sdk/pull/438 https://github.com/LIT-Protocol/js-sdk/pull/436, so we can run the tests as they rely on the new features introduced in those branches.

Is it juts a merged PR? So what's to review here? @Ansonhkg

The tests & the helper function

Ansonhkg avatar Apr 25 '24 00:04 Ansonhkg

Responded to your comments & left a couple new comments. After these are addresses this is good to merge 😄 @Ansonhkg

DashKash54 avatar May 01 '24 06:05 DashKash54