Export device types and labels #1226
Conversation
UI flow not yet tested
|
|
||
| runner = CliRunner() | ||
| result = runner.invoke(elementsd, ["--no-mining", "--cleanuphard"]) | ||
| print(result.output) | ||
| if result.exception != None: | ||
| if "Couldn't find executable elementsd" in str(result.exception): |
There was a problem hiding this comment.
Well, that's a good point. In the mid-term, i think we should somehow create different labels for the tests and make it possible that people skip certain test-types. Did that e.g. here:
https://github.com/cryptoadvance/specter-desktop/blob/master/tests/test_wallet_manager.py#L13
Existing markers are defined in pytest-ini
https://github.com/cryptoadvance/specter-desktop/blob/master/pytest.ini#L5
You could then run the tests without the slow ones like this:
pytest -m "not slow"
Haven't figured out how devs could set their own default settings which would be a good thing to have, though.
What do you think about that approach?
And yes, i guess i need to do some better documentation about:
./tests/install_bitcoind.sh --elements compile
There was a problem hiding this comment.
The test suite runs in under 5 minutes on my old Macbook Air (that's with the two elements tests skipping themselves). So I'm not worried yet about getting finer control over the tests. I think the bigger issue is that we just need more tests! Stepan caught at least one bug in my code that should have ideally been caught by a test case.
There was a problem hiding this comment.
Yeah, more tests are always better. 5 mins is to me already too long. Also it's the distribution. On my nuc:
- pytest -m "not slow" --> 81 tests in 10s
- pytest --> 89 tests in 3:17 mins
|
Great PR! Awesome work! Explicitly i like the detailed description of your changes and even more the added tests. Again, this is raising the bar for all of us! Thank you very much! |
Co-authored-by: Kim Neunert <[email protected]>
Co-authored-by: Stepan Snigirev <[email protected]>
|
I'm afraid you need to add a few more |
Fixes #1176
Makes the first half of #1109 redundant.
Changes the Specter backup format to include more information about each signing device. As pictured above, even though each device is unknown to this Specter instance, the correct device types and names are applied in the import process.
Import is still fully backwards compatible with previous Specter backups that do not have these new fields.
Changes:
helpers.py: Pulls device types and labels from the new Specter backup file format as well as labels from Electrum backup files. Also fixes a bug when the Electrum file does not contain an "addresses" field. Changes wallet type determination to use thederivation pathSLIP-132 xpub rather than address prefixes. Allows for detection of more wallet types.wallets.py: Moved functionality out of the endpoint and intoDescriptor.parse_signersto make it easier to write test cases and just general better code cleanliness/separation of concerns. Escaping on the incoming wallet backup json was overly broad (e.g. the apostrophe in the wallet name was being escaped to "h"); now limited to just the "descriptor" field. Passes along any device types and names found in the backup file to the UI.import_wallet.jinja: Changes to use the device types and names when available.wallet.py: Changesaccount_mapgeneration to use built-in json rather than relying on escaping strings. Adds the new "devices" field to the backup.test_wallet_manager.py: Two extensive new test cases to verify that single sig and multisig wallet backups can be restored with or without the new "devices" field.Misc changes:
DeviceTypes(device_types.py) which is just a set of constants to avoid string-based device typing elsewhere in the code. Just a good habit to reference by constants instead of strings, especially as the list of device types keeps growing.node_controller.py: Adds additional time to the node timeout check if the node is still going through its initial launch steps. Only accounts for the two conditions I happened to hit ("Verifying blocks" and "Loading wallet") when running a node on my old MacBook Air. Likely that other messages from the node would signal that it's still working on spinning up. Also restored theatexitcall with new exception handling to ensure that the internal node is properly shut down, especially when running the tests.test_cli_bitcoind.pyandtest_node_controller.py: Workaround fix to skip test whenelementsis not installed.tests/utils.py: Didn't mean to check this in, but contains an initial reusable function (that isn't being used yet) that will be useful for more complicated spending-related test cases.