search

BlockchainCommons / seedtool-cli

Cryptographic Seed Tool for the command line
Other
29 stars 16 forks source link

Allow any combination of input format and output format. #69

Open Sjlver opened 2 years ago

Sjlver commented 2 years ago

There's no technical reason why this shouldn't be allowed.

Closes https://github.com/BlockchainCommons/seedtool-cli/issues/63

Sjlver commented 2 years ago

You need to add many more tests to prove that any input format to any output format also work.

That's a bit of a high standard, considering that there were no tests before ;-)

Anyway, I've added a testcase that tests all meaningful combinations of input/output formats. Let me know what you think.

The test might actually have discovered a bug; shouldn't the output of these two commands be the same? Note that the checksum words are different.

$ src/seedtool --deterministic TEST | src/seedtool --deterministic TEST --out sskr
tuna acid epic gyro many meow able able able next edge lamb liar city girl draw visa roof logo jolt city waxy jury trip dark safe arch flew what
$ src/seedtool --deterministic TEST | src/seedtool --deterministic TEST --in hex --out sskr
tuna acid epic gyro edge next able able able next edge lamb liar city girl draw visa roof logo jolt city waxy jury trip dark loud song main atom
ChristopherA commented 2 years ago

You need to add many more tests to prove that any input format to any output format also work.

That's a bit of a high standard, considering that there were no tests before ;-)

Anyway, I've added a testcase that tests all meaningful combinations of input/output formats. Let me know what you think.

I believe all the tests are here: https://github.com/BlockchainCommons/seedtool-cli/blob/master/src/test.sh

Sjlver commented 2 years ago

I have fixed the test now. What I had thought to be a bug was actually not a bug, just an artifact of using the same deterministic seed in two invocations of seedtool-cli.

@wolfmcnally what do you think? Would this be good enough or do you need more tests than that?