Reuse derived Shamir seed and split off test fixtures

[tsusanka]

Replacement for #1034, fixes #1007. Conflicts with #1096 most likely. Let's merge #1096 first and I will rebase on top of that later.

---

This moves the test data into a common folder which we can share with other projects. It also nicely cleans up the test files.

Regarding SLIP-39 I have indeed moved it into separate files as discussed on Slack and I think it is a good direction.

There are number of questions to discuss:

• Replace client.use_passphrase("passphrase") with setup_client(passphrase="passphrase") where applicable. We do not have to do it but it would be nice? Moved to Replace use_passphrase with setup_client in tests #1110.
• Should we have some default values? For example if mnemonic is not present than "all all all" would be assumed. I am bit against this because if we plan to use the fixtures by other projects all the information should be present.
• Replace the silly "woman abuse" with something else while we are at it. "all all all" I guess?
• Do we want to separate valid and invalid test cases? Again using .valid.json and .invalid.json. But is seems just fine the way it is now (together).
• Do we want to use something else than JSON? We could use YAML or various other formats. I am against something like "JSON with comments to be striped out" that's just weird. @mroz22 any takes on this?
• Support both arrays and non-arrays in parametrize_using_common_fixtures so both ["cardano/sign_tx.json"] and "cardano/sign_tx.json" is valid to ease the usage. Sounds reasonable?
• Discuss with Suite if this suites them. cc @mroz22 @szymonlesisz

[matejcik]

very nice overall

Regarding SLIP-39 I have indeed moved it into separate files as discussed on Slack and I think it is a good direction.

👍

• Replace client.use_passphrase("passphrase") with setup_client(passphrase="passphrase") where applicable. We do not have to do it but it would be nice?

👍
let's have the new guy do it ;))

• Should we have some default values? For example if mnemonic is not present than "all all all" would be assumed. I am bit against this because if we plan to use the fixtures by other projects all the information should be present.

Agreed, let's always fill the mnemonic.
in fact, let's always specify mnemonic and passphrase, use empty string for no passphrase.

• Replace the silly "woman abuse" with something else while we are at it. "all all all" I guess?

such program, very feature, much valid, dog useful perhaps? :)
but in practice yes, as long as we're regenerating testcases, let's make them all all all

• Do we want to separate valid and invalid test cases? Again using .valid.json and .invalid.json. But is seems just fine the way it is now (together).

We absolutely want these separate.
I didn't add a review comment about this. The reason to separate them is so that we can write test_failing() and test_succeeding() separately without ifs inside.

• Do we want to use something else than JSON? We could use YAML or various other formats. I am against something like "JSON with comments to be striped out" that's just weird. @mroz22 any takes on this?

No opinion. Personally i'd be OK with "JSON with comment fields".

• Support both arrays and non-arrays in parametrize_using_common_fixtures so both ["cardano/sign_tx.json"] and "cardano/sign_tx.json" is valid to ease the usage. Sounds reasonable?

better use *args

[tsusanka]

in fact, let's always specify mnemonic and passphrase, use empty string for no passphrase.

So currently, we support passphrase: bool or passphrase: str. So do you think we should use passphrase: str only? We might loose tests that do not enable passphrase then :/. Maybe we should indeed distinguish this into two fields use_passphrase and passphrase?

[matejcik]

in fact, let's always specify mnemonic and passphrase, use empty string for no passphrase.

So currently, we support passphrase: bool or passphrase: str. So do you think we should use passphrase: str only? We might loose tests that do not enable passphrase then :/. Maybe we should indeed distinguish this into two fields use_passphrase and passphrase?

passphrase: Optional[str]. if None, passphrase is disabled; if str, it's enabled.

there might be some test cases that want passphrase enabled but don't want the setup to call use_passphrase()? but I'm not sure whether it's worth supporting them separately. Depends on how many such there are. I'd expect about two, in which case they can enable passphrase via apply_settings call inside the testcase, or specify passphrase="" and override it, or something.

[matejcik]

worth considering: what if we just enabled passphrase always? tests that explicitly do not want that would then explicitly disable the setting.

[tsusanka]

Do we want to separate valid and invalid test cases? Again using .valid.json and .invalid.json. But is seems just fine the way it is now (together).

Any opinions on the names? I have added .failures. for now, maybe failed is better? Or the aforementioned invalid? I do not like that anymore it sounds like the json is invalid.

[matejcik]

Any opinions on the names? I have added .failures. for now, maybe failed is better? Or the aforementioned invalid? I do not like that anymore it sounds like the json is invalid.

OTOH anything with "failed" sounds like the tests are failing.

but i don't really have an opinion, either is fine.

[matejcik]

do we still need the success field if the vectors are in separate files?

[tsusanka]

Replaced by #1235.