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

test: pop up parachain #86

Merged
merged 29 commits into from
Apr 9, 2024
Merged

test: pop up parachain #86

merged 29 commits into from
Apr 9, 2024

Conversation

AlexD10S
Copy link
Collaborator

@AlexD10S AlexD10S commented Mar 26, 2024

  • Unit tests for pop up parachain.
    A couple of tests are under unit_parachain feature because takes a lot of time (Ideally slow process should be mocked for unit tests). To run the ignored tests: cargo test --features unit_parachain
    To run all the tests: cargo test --all-features.

- Integration test.
To have an integration test of this functionality is a bit tricky, as the command doesn't stop after the execution.
I took as a reference a test in cumulus that let the command run for a X amount of seconds and then stop it. (Example here).

@evilrobot-01
Copy link
Contributor

evilrobot-01 commented Mar 26, 2024

Something like https://stackoverflow.com/questions/67859926/how-to-run-cargo-fmt-on-save-in-vscode should save the need for formatting commits in future.

Copy link
Contributor

@evilrobot-01 evilrobot-01 left a comment

Choose a reason for hiding this comment

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

Great work and thank you!! Biggest distraction for me was the contract-related formatting changes which should not have been included in this PR. I understand why, but it should be out of scope of the changes included.

Apart from that, made some suggestions and comments. Also I ran a code coverage tool and there was only 70% test coverage on zombienet.rs, admittedly without the ignored tests which are now still re-running, so need to add more tests which we can also do in a follow up as we look to further improve test coverage across the codebase.

src/engines/contract_engine.rs Outdated Show resolved Hide resolved
src/engines/parachain_engine.rs Outdated Show resolved Hide resolved
tests/build_contract.rs Outdated Show resolved Hide resolved
tests/wrong_config_no_para_id.toml Outdated Show resolved Hide resolved
tests/wrong_config_no_relay.toml Outdated Show resolved Hide resolved
tests/up_parachain.rs Outdated Show resolved Hide resolved
tests/up_parachain.rs Outdated Show resolved Hide resolved
tests/up_parachain.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/parachains/zombienet.rs Outdated Show resolved Hide resolved
src/parachains/zombienet.rs Outdated Show resolved Hide resolved
src/commands/new/contract.rs Outdated Show resolved Hide resolved
src/parachains/zombienet.rs Outdated Show resolved Hide resolved
src/parachains/zombienet.rs Outdated Show resolved Hide resolved
src/parachains/zombienet.rs Outdated Show resolved Hide resolved
src/parachains/zombienet.rs Outdated Show resolved Hide resolved
src/parachains/zombienet.rs Outdated Show resolved Hide resolved
src/parachains/zombienet.rs Show resolved Hide resolved
tests/up_parachain.rs Outdated Show resolved Hide resolved
src/parachains/zombienet.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@brunopgalvao brunopgalvao left a comment

Choose a reason for hiding this comment

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

Looks like the CI is failing.

@AlexD10S
Copy link
Collaborator Author

AlexD10S commented Apr 8, 2024

Looks like the CI is failing.

Removed the test that was failing. It was failing because is expecting user confirmation to download the binaries. Locally works because this binaries exist in cache.

The removed tests was more than 1h, because was trying all the process: create, build and run parachain (And relay chain).

Recover the test when we start our e2e battery of tests: 50669f4

@AlexD10S AlexD10S requested review from brunopgalvao and weezy20 April 8, 2024 13:01
@AlexD10S AlexD10S merged commit 07c8414 into main Apr 9, 2024
1 check passed
@AlexD10S AlexD10S deleted the alex/test-up-parachain branch April 9, 2024 08:43
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.

4 participants