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.

Sign up for GitHub

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

Jump to bottom

Lightweight testlib #4622

Merged
merged 44 commits into from
Aug 12, 2021
Merged

Lightweight testlib #4622

merged 44 commits into from
Aug 12, 2021

Conversation

nikurt
Copy link
Contributor

@nikurt nikurt commented Aug 2, 2021

Remove the dependency of testlib on nearcore by extracting tests and code dependeing on nearcore into integration-tests.

nikurt and others added 28 commits July 26, 2021 15:29
@nikurt
Remove dependency of 'chain/client' on 'nearcore' by moving its tests…
f0224db 
… into 'integraton_tests'
@nikurt
Remove dependency of 'chain/client' on 'nearcore' by moving its tests…
49d328f 
… into 'integraton_tests'
@nikurt
Remove dependency of 'chain/client' on 'nearcore' by moving its tests…
9e29ada 
… into 'integraton_tests'
@nikurt
rustfmt
b5da0e7
@nikurt
better layout of near-client integration tests
f3d49f4
@nikurt
better layout of near-client integration tests
6166ca5
@nikurt
.
5073865
@nikurt
Merge branch 'master' into cycle-t4
aa081b7
@nikurt
.
4e6ecac
@nikurt
update nightly features of integration tests
49eb3a5
@nikurt
merged
8d0f758
@nikurt
Merged
1ae217a
@nikurt
metric_recorder feature
110b417
@nikurt
Merge branch 'master' of https://github.com/near/nearcore into cycle-n
15e848b
@nikurt
Remove a 'dev-dependency' of 'near-client' on 'testlib' to prevent ci…
d923af6 
…rcular dependencies.
@nikurt
Merge branch 'master' into cycle-n
df0b1b4
@nikurt
Remove dependency of 'chain/client' on 'nearcore' by moving its tests…
0d2e2fa 
… into 'integraton_tests'
@nikurt
Remove dependency of 'chain/client' on 'nearcore' by moving its tests…
4b8cd31 
… into 'integraton_tests'
@nikurt
Remove dependency of 'chain/client' on 'nearcore' by moving its tests…
6179cba 
… into 'integraton_tests'
@nikurt
merge
2583ec3
@nikurt
merge
f31fafb
@nikurt
merge
e015d10
@nikurt
merge
a71fa25
@nikurt
merge
00b90b0
@nikurt
feature protocol_feature_block_header_v3 is required to run the new i…
5be00c2 
…ntegration tests
@nikurt
Move test_evil_contracts test into integration tests, as it's the onl…
618006b 
…y dependency on RuntimeNode outside of integration tests
@nikurt
Extract crate 'testlib::node' into 'integration_tests' and move tests…
327b601 
… depending on it into 'integration-tests', also move 'testlib_test_helpers'. 'testlib/run_nodes.rs' is special, turn it into a crate that depends on 'integration_tests'
@nikurt
Remove 'nearcore' dependency of 'testlib' by extracting functions and…
ce8e9fd 
… tests into 'integration-tests'
@nikurt nikurt requested a review from matklad August 2, 2021 16:57
@nikurt nikurt marked this pull request as ready for review August 10, 2021 12:23
nikurt and others added 5 commits August 10, 2021 14:31
@nikurt
Merge remote-tracking branch 'origin/master' into testlib-1
0a6e1bd 
# Conflicts:
#	Cargo.lock
#	integration-tests/Cargo.toml
#	integration-tests/tests/client/chunks_management.rs
#	integration-tests/tests/test_errors.rs
#	runtime/runtime/src/state_viewer/mod.rs
#	test-utils/testlib/src/lib.rs
#	test-utils/testlib/src/runtime_utils.rs
@nikurt
Merge
ab3014b
@nikurt
Merge
c1bdd06
@nikurt
Merge
cef5e2b
@nikurt
Merge branch 'master' into testlib-1
18ca98b
frol
frol approved these changes Aug 10, 2021
View reviewed changes
Copy link
Collaborator

@frol frol left a comment

Choose a reason for hiding this comment

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

I see many things move around. The whole reorg makes total sense to me, but I fear I could miss some minor details among the changes; are there any?

Also, can we measure the wins that we achieve with this reorg (e.g. compilation / testing times)? I would love to see those

@frol
Copy link
Collaborator

frol commented Aug 10, 2021

Just to bring some color here, @miraclx is also working on quality of life improvements for nearcore codebase, and here is a recent win: #4651 (comment)

@nikurt Thanks for pulling this out!

bowenwang1996
bowenwang1996 approved these changes Aug 11, 2021
View reviewed changes
test-utils/run-nodes/Cargo.toml Outdated Show resolved Hide resolved
@nikurt
Copy link
Contributor Author

nikurt commented Aug 11, 2021

Just to bring some color here, @miraclx is also working on quality of life improvements for nearcore codebase, and here is a recent win: #4651 (comment)
Thanks for the link. Now I'm also curios, will measure the impact and report here.

@nikurt
Copy link
Contributor Author

nikurt commented Aug 11, 2021

minor details among the changes; are there any?
I tried to keep the PR as simple as possible, without making any changes unrelated to the refactoring. The worst that could happen is an artifact of merging, which hopefully is caught by buildkite.

@nikurt nikurt mentioned this pull request Aug 11, 2021
Merged
@nikurt
Copy link
Contributor Author

nikurt commented Aug 11, 2021

Created #4670 to remove run_nodes.rs.

@nikurt nikurt linked an issue Aug 11, 2021 that may be closed by this pull request
Consider removing circular dependencies during tests #4357
Closed
@nikurt
Copy link
Contributor Author

nikurt commented Aug 11, 2021

Just to bring some color here, @miraclx is also working on quality of life improvements for nearcore codebase, and here is a recent win: #4651 (comment)
Thanks for the link. Now I'm also curios, will measure the impact and report here.

cargo test -p node-runtime
debug clean build time: 4m 30s / 4m 20s
debug binary size: 78 MB / 71 MB
dependencies: 612 / 591
debug test runtime: 6m 0s / 30s

cargo test -p nearcore
debug clean build time: 4m 46s / 4m 9s
debug binary size: 96M / 96M
dependencies: 616 / 608
debug test runtime: 9m 50s / 5s

near-bulldozer bot pushed a commit that referenced this pull request Aug 11, 2021
@nikurt
Remove run_nodes.rs (#4670)
9f5e20b 
Remove `run_nodes.rs`, which is a standalone binary built as part of the `testlib` crate. No usages of `run_nodes` were identified in `near-ops`, `nearcore` or `nearup` repositories.

Part of PR #4622
@nikurt
Merge remote-tracking branch 'origin/master' into testlib-1
4eafb84 
# Conflicts:
#	test-utils/run-nodes/src/main.rs
@near-bulldozer near-bulldozer bot merged commit fd7bcc0 into master Aug 12, 2021
@near-bulldozer near-bulldozer bot deleted the testlib-1 branch August 12, 2021 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bowenwang1996 bowenwang1996 bowenwang1996 approved these changes

@matklad matklad matklad approved these changes

@frol frol frol approved these changes

@EgorKulikov EgorKulikov Awaiting requested review from EgorKulikov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Consider removing circular dependencies during tests
4 participants
@nikurt @frol @matklad @bowenwang1996