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

Enable tests in safe_core::client::mock - Part 2 #1109

Merged
merged 1 commit into from Dec 12, 2019

Conversation

@Yoga07
Copy link
Member

Yoga07 commented Dec 10, 2019

Re-enables 2nd half of the mock vault tests in safe_core.
Rebased on: #1106

@Yoga07 Yoga07 requested review from m-cat and lionel1704 Dec 10, 2019
@Yoga07 Yoga07 requested a review from nbaksalyar as a code owner Dec 10, 2019
@Yoga07 Yoga07 force-pushed the Yoga07:mock-net-tests branch from 6dd3cc6 to 4488226 Dec 10, 2019
Copy link
Member

lionel1704 left a comment

Nice one @Yoga07
I've left some comments. There are a few places where we can use the macro. It will reduce the verbosity of the test code. process_request can be used only when we need to do validations on a successful response.

Are you planning to refactor the other tests in the same PR? Or will that be a separate one?

safe_core/src/client/mock/tests.rs Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Show resolved Hide resolved
@Yoga07 Yoga07 force-pushed the Yoga07:mock-net-tests branch 2 times, most recently from 32bbec8 to 4c37668 Dec 11, 2019
@Yoga07

This comment has been minimized.

Copy link
Member Author

Yoga07 commented Dec 11, 2019

Are you planning to refactor the other tests in the same PR? Or will that be a separate one?

No @lionel1704 , I've rebased and pushed a few more changes now which brings back the rest of the tests as well.

safe_core/src/client/mock/connection_manager.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/vault.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/vault.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/vault.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
@Yoga07 Yoga07 force-pushed the Yoga07:mock-net-tests branch from 4c37668 to 3753e34 Dec 12, 2019
@Yoga07 Yoga07 requested review from m-cat and lionel1704 Dec 12, 2019
Copy link
Member

lionel1704 left a comment

Nice one @Yoga07
Just a few comments.

safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
safe_core/src/client/mock/tests.rs Outdated Show resolved Hide resolved
@Yoga07 Yoga07 force-pushed the Yoga07:mock-net-tests branch from 3753e34 to 327c645 Dec 12, 2019
@Yoga07 Yoga07 requested a review from lionel1704 Dec 12, 2019
@Yoga07 Yoga07 force-pushed the Yoga07:mock-net-tests branch from 327c645 to 25e84a5 Dec 12, 2019
@lionel1704 lionel1704 dismissed m-cat’s stale review Dec 12, 2019

The comments have been addressed.

@lionel1704 lionel1704 merged commit f694747 into maidsafe:master Dec 12, 2019
14 checks passed
14 checks passed
Rustfmt-Clippy
Details
Build Scripts
Details
build-ios (aarch64-apple-ios) build-ios (aarch64-apple-ios)
Details
build-ios (x86_64-apple-ios) build-ios (x86_64-apple-ios)
Details
build-android (armv7-linux-androideabi, prod)
Details
build-android (armv7-linux-androideabi, dev)
Details
build-android (x86_64-linux-android, prod)
Details
build-android (x86_64-linux-android, dev)
Details
Test (ubuntu-latest)
Details
Test (windows-latest)
Details
Test (macOS-latest)
Details
Build iOS Universal (prod)
Details
Build iOS Universal (dev)
Details
Travis CI - Pull Request Build Passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.