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

Add Makefile, bash scripts to test multiple nodes #20

Merged
merged 40 commits into from
May 22, 2022
Merged

Add Makefile, bash scripts to test multiple nodes #20

merged 40 commits into from
May 22, 2022

Conversation

asalzmann
Copy link
Contributor

@asalzmann asalzmann commented May 17, 2022

Summary
Add Makefile, scripts to run multiple nodes. Before testing ICA / ICQ, we'll need scripts to bootstrap validator nodes and send calls between them.

This is pulled from the Osmosis repo.

It doesn't quite work yet, possible we need to reconfigure something with docker

Test plan
make build and make test

@asalzmann asalzmann changed the title Test 35 Add Makefile, bash scripts to test multiple nodes May 17, 2022
strided keys add validator3 --keyring-backend=test --home=$HOME/.strided/validator3

# change staking denom to uosmo
cat $HOME/.strided/validator1/config/genesis.json | jq '.app_state["staking"]["params"]["bond_denom"]="uosmo"' > $HOME/.strided/validator1/config/tmp_genesis.json && mv $HOME/.strided/validator1/config/tmp_genesis.json $HOME/.strided/validator1/config/genesis.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be STRD? does it matter for testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think it matters too much for testing but great catch, we should change this to STRD

@@ -98,13 +96,13 @@ func (k msgServer) MintStAsset(ctx sdk.Context, sender sdk.AccAddress, amount in
// It will panic if the module account does not exist or is unauthorized.
sdkerror := k.bankKeeper.MintCoins(ctx, types.ModuleName, stCoins)
if sdkerror != nil {
k.Logger(ctx).Info("Failed to mint coins");
Copy link
Contributor

@riley-stride riley-stride May 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

were these breaking with semicolons at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was a linter change

_ servertypes.Application = (*App)(nil)
_ simapp.App = (*App)(nil)
_ servertypes.Application = (*StrideApp)(nil)
_ ibctesting.TestingApp = (*StrideApp)(nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did we add this back?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is part of Vishal's PR #14, had to merge it to this branch in order to get the build working (although now that #14 is merged into main, this change should disappear once this branch is merged to main)

benchmark \
build-docker-osmosisdnode localnet-start localnet-stop \
docker-single-node

test:
Copy link
Contributor

@riley-stride riley-stride May 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The makefile now has three "test" targets.

  • The first and second define recipes which run all the tests in the packages within the ./x/ folder and unit tests (many of which fail, as expected, since we haven't written these yet; I'm holding these tests are placeholders for now)
    • The second runs unit tests too through test-unit dependency. This seems to run the same tests as the first recipe, but it breaks on error. I'm commenting this out for now to get things running since we wouldn't expect these unit tests to work, so we would not want to fail on them until we've written them properly. Added TODO(TEST-49) to write the unit tests properly.
    • It also tries to run a test-build dependency which doesn't exist (?)
  • The third runs script/simple_test.sh, which works after I made a tweak to reset blockchain state (added the --reset-once flag

Is your intention to run the tests in each folder of the project or to run simple_test.sh? @asalzmann

- commented out test-unit in Makefile (we haeven't written unit tests yet!)
- added blockchain reset arg to scripts/simple_test.sh
@riley-stride
Copy link
Contributor

Some notes from fiddling with this:

Issue 1: Validator 1 was silently dying after launching it on line 112

  • Validator 1 tmux session was dying upon initialization because another program was already listening at port 6060.
  • Check which process it is with
    sudo lsof -i -P -n | grep 6060
  • Kill the process with:
    kill -9 <PID>
  • Same issue with port 26656, repeat the fix

Same issue was occurring between runs because the old tmux processes were still running. An easy way to fix:

pkill strided

Helpful bash commands:

  • Tmux list-sessions
  • Tmux a
  • (Once in tmux session, how to disconnect) ctrl+b d

@asalzmann
Copy link
Contributor Author

let's go
Screen Shot 2022-05-22 at 12 47 16 AM

@asalzmann asalzmann merged commit d5a698b into main May 22, 2022
asalzmann pushed a commit that referenced this pull request Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants