Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LCD cleanup and tests refactor #624

Closed
ebuchman opened this issue Mar 15, 2018 · 1 comment
Closed

LCD cleanup and tests refactor #624

ebuchman opened this issue Mar 15, 2018 · 1 comment
Labels

Comments

@ebuchman
Copy link
Member

ebuchman commented Mar 15, 2018

@faboweb made some heroic efforts to get the LCD in place: #544

That said, the client/lcd/lcd_test.go is using the unreviewed/unfinished integration tests from #596. These are integration tests that are written in Go to call the basecoind and basecli binaries - we shouldn't be using them to test the base functionality of the LCD.

So, some work needs to be done to spin up the Basecoind and LCD nodes in-process, without using the binaries at all. This requires some refactoring across the LCD to remove side-effects and dependence on global variables like viper.

I've taken a first stab at this in #625 . It doesn't compile yet and needs some love. We need to do things like setup the Keybase once up front and pass it through rather than relying on the side-effecting/global GetKeybase function called everywhere.

Would greatly appreciate some help here. It's a priority to get this code into high quality maintainable shape and it's not Fabo's responsibility to do so.

To be clear we should remove the tests package completely from develop in this excercise as there's no need for it at all for the LCD unit tests. We will review and merge that package independently from PR #596

@ebuchman
Copy link
Member Author

Mostly done. LCD/CLI still needs some serious love, but will leave it to other issues

MalteHerrmann pushed a commit to MalteHerrmann/cosmos-sdk that referenced this issue Oct 1, 2024
* regenerate addrbook.json

* lint

* change perms
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant