Additional Creditcoin js examples #1346
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #1346 +/- ##
===========================================
- Coverage 71.06% 13.86% -57.20%
===========================================
Files 107 31 -76
Lines 12537 988 -11549
Branches 126 126
===========================================
- Hits 8909 137 -8772
+ Misses 3618 851 -2767
+ Partials 10 0 -10 see 83 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
For full LLVM coverage report click here! |
creditcoin-js/README.md
Outdated
| ```typescript | ||
| import { personalSignSignature } from 'creditcoin-js/lib/extrinsics/register-address-v2'; | ||
| import { personalSignAccountId } from 'creditcoin-js/lib/utils'; | ||
| import { Wallet } from "creditcoin-js"; |
There was a problem hiding this comment.
Can we add this as files into the examples/ directory? In the past we tried executing them in CI to make sure they still work for example https://github.com/gluwa/creditcoin/blob/dev/integration-tests/src/test/loan-cycle.test.ts
There was a problem hiding this comment.
I see examples have been added but README still contains big chunks of code, which people will copy over.
My intention was that code blocks inside README will be replaced with something like
"see src/examples/collect-coins-v2.ts" for anything which is bigger than 5 lines.
…ress and collect coins
| @@ -107,4 +111,39 @@ describe('Test GATE Token', (): void => { | |||
| }, | |||
| 900_000, | |||
| ); | |||
|
|
|||
| // This test must run after the end to end test | |||
| // We are relying on the gate contract already being set, acceptable assumption since we run tests with --runInBand | |||
There was a problem hiding this comment.
I think this requirement is no longer needed b/c the gate contract is now set in beforeAll.
In addition eventhough we use --runInBand I'm not quite sure how reliable that is, however that's a side topic.
FTR: I've started prefixing tests with a numeric prefix if the order matters, e.g. 001 - end-to-end inside collect-coins.
There was a problem hiding this comment.
I misspoke in the comments. We need the gate contract to be set AND the faucet account to be set. The first sub-test of the e2e test is whether the extrinsic correctly reports that the faucet hasn't yet been set. After the error has been reported we set the faucet and try again this time expecting a success. So we need that sub-test to run before the example
| ) { | ||
| const { api } = ccApi; | ||
|
|
||
| const contract = api.createType('PalletCreditcoinOcwTasksCollectCoinsDeployedContract', { |
There was a problem hiding this comment.
Can we get some comments here about what the createType call is doing and how the string argument was determined? It's a bit ambiguous for someone trying to learn to use the library.
| import { CollectCoinsContract } from '../model'; | ||
| import { KeyringPair } from '@polkadot/keyring/types'; | ||
|
|
||
| export async function collectCoinsV2Example( |
There was a problem hiding this comment.
I think for all of these examples, it would be best if they were written expecting no arguments. The examples original provided by Pablo all follow the same signature of an async function with no parameters, and I think we should maintain that where we can.
There was a problem hiding this comment.
That would be possible for the register-address and set-collect-coins-contract examples because they are light on dependencies. The collect coins example needs the fully deployed token setup and also calls to setGateContract and setGateFaucet. I think all that extra code might take away from the point of the example.
Needs to be rebased on the PR for branch 256-ccjs-docs when that is finished