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

Integration tests #4

Merged
merged 21 commits into from
Jan 20, 2023
Merged

Conversation

mariusmihaic
Copy link
Contributor

@mariusmihaic mariusmihaic commented Dec 28, 2022

Added integration tests scripts to start firehose ingestion nodes for metachain and shard with a local testnet automatically and check indexed data validity for custom scenario(ESDTIssue).

Please note: This code(go) shall be reviewed as a testing framework code, not as a productive code. This is the reason why many values are hard-coded and certain variables are expected to be set to a specific value

Scripts:

  • main.go contains the main scenario. It will:
  1. Load an address with with balance from the local testnet setup
  2. Send an ESDT issue tx and get its hash
  3. Depending on meta/shard id, check the block in which the hash was included and start validity data checks. For shard, it has to do an additional search for the hyperBlock to find in which block was included.
  • local-testnet.sh - adaptation from this script to create/use a local testnet
  • integration-test.sh - setup for a new firehose node + ./start.sh call to start indexer node(which will internally start a multiversx node- shard/meta; only 1 shard for current config)
  • shard-meta-tests.sh - starts with screen a new firehose+observer node in meta/shard id and calls go script(main.go after building) for the custom scenario.

@mariusmihaic mariusmihaic changed the base branch from EN-13586-new-elrond-core to feat/integration January 5, 2023 11:58
@mariusmihaic mariusmihaic self-assigned this Jan 6, 2023
@mariusmihaic mariusmihaic marked this pull request as ready for review January 6, 2023 12:06
@mariusmihaic mariusmihaic changed the title En 13062 integration tests Integration tests Jan 6, 2023
@schimih schimih self-requested a review January 16, 2023 15:51
@ssd04 ssd04 self-requested a review January 16, 2023 16:22
Copy link

@schimih schimih left a comment

Choose a reason for hiding this comment

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

Functional it seems ok.
Just a general note: rebranding.

git checkout 4f82f49ede54e14b19f42028d910e41a728b3925
cd ../..

git clone https://github.com/ElrondNetwork/elrond-deploy-go "$TESTNET_DIR/elrond-deploy-go"
Copy link

Choose a reason for hiding this comment

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

For what is elrond-deploy-go needed in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, good catch.
It is not needed. Removed it


shardBlocks := gjson.Get(string(body), "data.hyperblock.shardBlocks").Array()
if len(shardBlocks) != 1 {
return fmt.Errorf("checkShardHeader: should only have one shard, but got %d", len(shardBlocks))
Copy link

Choose a reason for hiding this comment

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

should only have one shard or block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One shard only. It is described in the app usage and script usage as well:

script:
This script will start a firehose node in the provided shard(metachain/shard). The only acceptable parameters are shard(will use shard 0) and metachain for the current test configuration

checker:
app.Usage = "This tool only works if a local testnet and a firehose node are started. See firehose-node.sh and local-testnet.sh scripts"

Will also add a Readme here to make things clearer

return checkShardAlteredAccounts(multiversxBlock.MultiversxBlock.AlteredAccounts, address)
}

func checkShardHeader(multiversxBlock *firehose.FirehoseBlock, shardBlocks []gjson.Result) error {
Copy link

Choose a reason for hiding this comment

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

Suggested change
func checkShardHeader(multiversxBlock *firehose.FirehoseBlock, shardBlocks []gjson.Result) error {
func checkShardBlockHeader(multiversxBlock *firehose.FirehoseBlock, shardBlocks []gjson.Result) error {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, changed as suggested

return fmt.Errorf("checkShardTxs: invalid computed tx hash, expected: %s, got %s", txHash, txProtocolHexHash)
}

initialPaidFeed := protocolTx.FeeInfo.GetInitialPaidFee()
Copy link

Choose a reason for hiding this comment

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

Suggested change
initialPaidFeed := protocolTx.FeeInfo.GetInitialPaidFee()
initialPaidFee := protocolTx.FeeInfo.GetInitialPaidFee()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, good catch, changed it.


initialPaidFeed := protocolTx.FeeInfo.GetInitialPaidFee()
expectedInitialPaidFee := big.NewInt(txGasLimit * 1000000000)
if initialPaidFeed.Cmp(expectedInitialPaidFee) != 0 {
Copy link

Choose a reason for hiding this comment

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

Suggested change
if initialPaidFeed.Cmp(expectedInitialPaidFee) != 0 {
if initialPaidFee.Cmp(expectedInitialPaidFee) != 0 {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored

"github.com/ElrondNetwork/elrond-sdk-erdgo/interactors"
)

func getAddressAndSK(pemPath string) (core.AddressHandler, []byte, error) {
Copy link

Choose a reason for hiding this comment

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

Suggested change
func getAddressAndSK(pemPath string) (core.AddressHandler, []byte, error) {
func getAddressAndPK(pemPath string) (core.AddressHandler, []byte, error) {

Copy link
Contributor Author

@mariusmihaic mariusmihaic Jan 20, 2023

Choose a reason for hiding this comment

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

Pk is also an acronym for publicKey.
Meanwhile, Sk stands only for secretKey, so I'd leave it as it is.

Comment on lines 8 to 11
cd testnet/elrond-go/cmd/keygenerator
go build
./keygenerator
cd ../../../../../devel/standard/
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion to use pushd and popd here instead of so many ../..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed as suggested

@@ -0,0 +1,122 @@
CURRENT_DIR=$(pwd)
Copy link
Contributor

Choose a reason for hiding this comment

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

add #!/usr/bin/env bash as first line for the main script

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, added it

@@ -0,0 +1,56 @@
CURRENT_DIR=$(pwd)
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion to separate shell scripts and go code into separate folders, it's quite difficult to identify what to run and how, maybe add the info regarding scripts from PR description into a small README.md inside scripts folder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied suggestion and split code in different folders.
Also added a README.md

@mariusmihaic mariusmihaic merged commit 83aa06b into feat/integration Jan 20, 2023
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