QA file-restructure #247
QA file-restructure #247
Conversation
mixmix
commented
Oct 1, 2024
mixmix
commented
- a script for testing CLI
- some gotchas!
- WIP bugs!
tests/cli.test.sh
Outdated
| @@ -0,0 +1,53 @@ | |||
| #! /usr/bin/bash | |||
There was a problem hiding this comment.
Here's a script I've been using to QA the CLI
It's breaking down at Transfer 😠
src/transfer/command.ts
Outdated
| const entropy = await loadEntropy(opts.account, opts.endpoint) | ||
| .catch(err => { | ||
| // WIP here. SOMETHING is wrecking the config upstream | ||
| console.error("loadEntropy failed", err) | ||
| throw err | ||
| }) |
| import { EntropyTransfer } from "./main" | ||
|
|
||
| export function entropyTransferCommand () { | ||
| const transferCommand = new Command('tranfer') | ||
| const transferCommand = new Command('transfer') |
tests/cli.test.sh
Outdated
| # entropy balance faucet | ||
| entropy balance 5CqJyjALDFz4sKjQgK8NXBQGHCWAiV63xXn2Dye393Y6Vghz | ||
|
|
||
|
|
There was a problem hiding this comment.
maybe we should log the config through-out this script to see where it done breaks
| const accounts = await config.get() | ||
| .then(storedConfig => EntropyAccount.list(storedConfig)) | ||
| .catch((err) => { | ||
| if (err.message.includes('currently no accounts')) return [] | ||
|
|
||
| throw err | ||
| }) | ||
|
|
There was a problem hiding this comment.
entropy account ls
If I don't have any accounts yet I don't expect an error, I expect []
| @@ -35,7 +38,8 @@ function entropyAccountCreate () { | |||
|
|
|||
| cliWrite({ | |||
| name: newAccount.name, | |||
| address: newAccount.address | |||
| address: newAccount.address, | |||
| verifyingKeys: [] | |||
There was a problem hiding this comment.
entropy account create
entropy account create
Have them output the same format as entropy account ls
✔️ consistency
✔️ set up expectations that there is something interesting/ important meant to go in here
| const fullAccount = keyring.getAccount() | ||
| // TODO: sdk should create account on constructor | ||
| const { admin } = keyring.getAccount() | ||
| const data = fixData(fullAccount) |
There was a problem hiding this comment.
There is a bug in the SDK where you are "cloning" with `JSON.parse(JSON.stringify(obj)) and it fucks up the Uint8Array, which then fucks up config, logging etc.
This is a quick hack
| @@ -49,7 +49,7 @@ export class EntropyAccount extends EntropyBase { | |||
| return accounts.map((account: EntropyAccountConfig) => ({ | |||
| name: account.name, | |||
| address: account.address, | |||
| verifyingKeys: account?.data?.admin?.verifyingKeys | |||
| verifyingKeys: account?.data?.registration?.verifyingKeys || [] | |||
There was a problem hiding this comment.
🔥 CHECK?
I assume verifyingKeys were meant to go under registration and not admin right?
There was a problem hiding this comment.
AH tests say no. I don't know what the difference is between `admin + registration.
I changed where vks are being pushed into elsewhere in code too
| 'seed', | ||
| 'secretKey', | ||
| 'addressRaw', | ||
| ]; | ||
| ]); | ||
|
|
||
| export function maskPayload (payload: any): any { |
There was a problem hiding this comment.
I ended up in here cos of the masking of the badly handled UInt8Arrays... and reading it I saw there was not test and it was pretty confusing, so I wrote tests and wrote a simpler function with less dependencies.
I think it's an improvement ... I mean it needed to check the DEFAULT_MASKED_FIELDS earlier cos the test for "is this a Uint8Array or a munted Uint8Array (object like { "0": 34, "1": 23, ... }) ... yeah that broke when I fixed the bad keys coming from SDK...
There's a chance I introduced more edge cases...but look at the tests I guess!
| @@ -20,7 +20,7 @@ function getConfigOrNull () { | |||
|
|
|||
| export function endpointOption () { | |||
| return new Option( | |||
| '-e, --endpoint <endpoint>', | |||
| '-e, --endpoint <url>', | |||
There was a problem hiding this comment.
reading the docs the pattern is -shortcode --longcode <input-type>
| selectedAccount: account | ||
| }) | ||
| } | ||
| .argParser(addressOrName => { |
There was a problem hiding this comment.
🔥 🔥 NASTY bug
this thing don't handle async!
See comments below
There was a problem hiding this comment.
I caught this cos I tried to register naynay in my QA and it was registering faucet!
| export async function setSelectedAccount (account: EntropyAccountConfig, configPath = CONFIG_PATH) { | ||
| const storedConfig = await get(configPath) | ||
|
|
||
| if (storedConfig.selectedAccount === account.name) return storedConfig | ||
| // no need for update | ||
|
|
||
| const newConfig = { | ||
| ...storedConfig, | ||
| selectedAccount: account.name | ||
| } | ||
| await set(newConfig, configPath) | ||
| return newConfig | ||
| } | ||
|
|
There was a problem hiding this comment.
🔥 one bug I saw was inconsistency around what config.selectedAccount was (an address? a name?).
I decided to fix it to be name because then in the --help you will see the default by name not some crazy address
I then changed
accountOptionto deal withname- added a special method
setSelectedAccountto handle the persistence of the right thing
Things I'm not happy about:
- we are now doing a LOT of get/set/get/set of the same config in a few places
- some places we need to persist the whole config (e.g. new
verifyingKeys... and so the knowledge of what theselecttedAccountis is leaking out into the code) - does this suggest there should be a
getSelectedAccount?? where do the helper methods stop!
| # WARNING - this script nukes your config! | ||
| # | ||
| # Dependencies | ||
| # - internet connection | ||
| # - jq - see https://jqlang.github.io/jq | ||
| # | ||
| # Run | ||
| # $ yarn build && ./tests/qa.sh |
There was a problem hiding this comment.
I'm pretty stoked with this. This script allowed me to hammer the CLI repeatedly till I got all the weird lil bugs out.
I imagine this could be the basis for a CLI integration test that uses frankie's new CLI runner in the future so we can check the outputs are good
Checklist
|
