Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

Aura test update (master)#6115

Closed
General-Beck wants to merge 5 commits into
masterfrom
aura-test-master
Closed

Aura test update (master)#6115
General-Beck wants to merge 5 commits into
masterfrom
aura-test-master

Conversation

@General-Beck
Copy link
Copy Markdown
Contributor

No description provided.

@General-Beck General-Beck added A0-pleasereview 🤓 Pull request needs code review. M0-build 🏗 Building and build system. labels Jul 21, 2017
@arkpar
Copy link
Copy Markdown
Collaborator

arkpar commented Jul 21, 2017

This test should use a temp data dir which should be cleared before the execution

Comment thread scripts/aura-test.sh Outdated
#!/bin/bash
cargo build -j $(nproc) --release --features final $CARGOFLAGS
rm -rf parity-import-tests/
cargo build -j $(nproc) --release --features final
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no need for features final, this is not a public build

Comment thread scripts/aura-test.sh Outdated
rm -rf parity-import-tests/
cargo build -j $(nproc) --release --features final
git clone https://github.com/paritytech/parity-import-tests
cp target/release/parity parity-import-tests/aura/parity
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no need to copy the binary

@arkpar arkpar added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 21, 2017
Comment thread scripts/aura-test.sh Outdated
cargo build -j $(nproc) --release --features final $CARGOFLAGS
rm -rf parity-import-tests/
cargo build -j $(nproc) --release
git clone https://github.com/paritytech/parity-import-tests
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should really be a submodule

Comment thread scripts/aura-test.sh Outdated
echo "Start Aura test"
parity import blocks.rlp --chain chain.json
parity restore snap --chain chain.json
target/release/parity import parity-import-tests/aura/blocks.rlp --chain parity-import-tests/aura/chain.json
Copy link
Copy Markdown
Collaborator

@arkpar arkpar Jul 21, 2017

Choose a reason for hiding this comment

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

-d /tmp/aura-test-data

Comment thread scripts/aura-test.sh Outdated
echo "Import test failed" >&2
exit 1
fi
target/release/parity restore parity-import-tests/aura/snap --chain parity-import-tests/aura/chain.json
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

-d /tmp/aura-test-data

Comment thread scripts/aura-test.sh Outdated
@@ -1,9 +1,23 @@
#!/bin/bash
cargo build -j $(nproc) --release --features final $CARGOFLAGS
rm -rf parity-import-tests/
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Datadir must be cleared as well.
rm -rf /tmp/aura-test-data

@rphmeier
Copy link
Copy Markdown
Contributor

rphmeier commented Jul 21, 2017

In the future, there will be multiple tests in the parity-import-tests repo. After #6085 lands, I will add one in the tendermint directory. The future-proof solution would be to iterate over each directory in the repo and perform the import test commands for all of them.

We may also add tests for more features as they come up: those using WASM, those running PoS engines, etc.

@keorn keorn added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Jul 24, 2017
Comment thread scripts/aura-test.sh Outdated
git clone https://github.com/paritytech/parity-import-tests
cp target/release/parity parity-import-tests/aura/parity
cd parity-import-tests/aura
rm -rf parity-import-tests
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not needed anymore

Comment thread scripts/aura-test.sh
echo "Start Aura test"
parity import blocks.rlp --chain chain.json
parity restore snap --chain chain.json
target/release/parity import test/parity-import-tests/aura/blocks.rlp --chain test/parity-import-tests/aura/chain.json -d /tmp/aura-test-data
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@General-Beck there are now multiple import tests. Can you rewrite this as a loop to be future-proof?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@rphmeier What condition can be taken for a loop?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pseudocode like this

for test in parity-import-tests/*
  cd parity-import-tests/$test
  parity --chain chain.json -d /tmp/import-tests import blocks.rlp
  parity --chain chain.json -d /tmp/import-tests restore snap
  cd ../..

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

each directory has 3 files:

  • chain.json
  • blocks.rlp
  • snap

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@rphmeier Will there be a given amount of tests?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

no, there is no cap. we will add them periodically, whenever necessary.

@keorn keorn added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 22, 2017
@gavofyork
Copy link
Copy Markdown
Contributor

@General-Beck anything happening here?

@gavofyork gavofyork closed this Sep 26, 2017
@rphmeier
Copy link
Copy Markdown
Contributor

this should be revived @paritytech/core-devs. IMO integration tests for varying chains are a pretty critical part of our test infrastructure.

@5chdn 5chdn deleted the aura-test-master branch January 3, 2018 19:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. M0-build 🏗 Building and build system.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants