Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

(some) node subcommands are broken #4623

Closed
tomusdrw opened this issue Jan 14, 2020 · 8 comments · Fixed by #4634
Closed

(some) node subcommands are broken #4623

tomusdrw opened this issue Jan 14, 2020 · 8 comments · Fixed by #4634
Assignees
@cecton
Labels
I5-tests Tests need fixing, improving or augmenting. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.
Milestone
2.0

Comments

@tomusdrw
Copy link
Contributor

...and they fail with keystore not being configured. Namely import-blocks and factory at least.

Also it would be nice to actually write some tests for the sub-commands so that we don't run into that issue in the future.
@tomusdrw tomusdrw added I3-bug The node fails to follow expected behavior. M4-core Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder labels Jan 14, 2020
@tomusdrw tomusdrw added this to the 2.0 milestone Jan 14, 2020
@NikVolf NikVolf self-assigned this Jan 15, 2020
@NikVolf NikVolf mentioned this issue Jan 16, 2020
Merged
@bkchr
Copy link
Member

bkchr commented Jan 16, 2020

Part 1 is done and the CLI commands are fixed, now we need some tests.

@bkchr bkchr reopened this Jan 16, 2020
@bkchr bkchr added I5-tests Tests need fixing, improving or augmenting. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. and removed I3-bug The node fails to follow expected behavior. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder labels Jan 16, 2020
@bkchr
Copy link
Member

bkchr commented Jan 16, 2020

@NikVolf you also want to write the tests?

@NikVolf
Copy link
Contributor

NikVolf commented Jan 16, 2020

Yes @bkchr

@NikVolf
Copy link
Contributor

NikVolf commented Jan 20, 2020

I'll wait until @cecton finishes some refactoring there though
Test would be as silly as fix with current api 🤷‍♀️

@tomusdrw
Copy link
Contributor Author

@NikVolf maybe we could have tests that actually start a CLI? Then it's not really related to the API at all and we would test the CLI parsing in one go.

@cecton
Copy link
Contributor

cecton commented Jan 20, 2020

Definitely agree! I made the PR already, I will add some tests tomorrow. #4692

@NikVolf NikVolf assigned cecton and unassigned NikVolf Feb 12, 2020
@cecton
Copy link
Contributor

cecton commented Feb 12, 2020

Fixing it in #4842

@gnunicorn
Copy link
Contributor

is this after the merge of #4842 still open?

@bkchr bkchr closed this as completed Feb 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@cecton cecton

Labels
I5-tests Tests need fixing, improving or augmenting. Z2-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase.
Projects
None yet
Milestone
2.0
Development

Successfully merging a pull request may close this issue.

Use default in-memory keystore for commands that don't use keystore paritytech/substrate
5 participants
@gnunicorn @tomusdrw @cecton @NikVolf @bkchr