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

Add test for feature RPC (RIPD-1391): #1988

Closed
wants to merge 1 commit into
base: develop
from

Conversation

Projects
None yet
5 participants
@mellery451
Contributor

mellery451 commented Jan 30, 2017

Create unit test for feature RPC method. Add client_error field to env
RPC requests to provide information about parsing errors.

@mellery451

This comment has been minimized.

Show comment
Hide comment
@mellery451

mellery451 Feb 1, 2017

Contributor

the drop on coverage is because sweepTimer went off (unexpectedly) in 1ede097. I'll be dealing with that in a separate PR.

Contributor

mellery451 commented Feb 1, 2017

the drop on coverage is because sweepTimer went off (unexpectedly) in 1ede097. I'll be dealing with that in a separate PR.

@nbougalis

Left a small comment for one extra test, but it's fine as is: 👍

auto jrr = env.rpc("feature", "AllTheThings") [jss::result];
BEAST_EXPECT(jrr[jss::error] == "badFeature");
BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid.");

This comment has been minimized.

@nbougalis

nbougalis Feb 3, 2017

Member

Maybe add one more test here: feature names are case-sensitive, so we could ensure that, say, CryptoConditions and cryptoconditions don't match up.

@nbougalis

nbougalis Feb 3, 2017

Member

Maybe add one more test here: feature names are case-sensitive, so we could ensure that, say, CryptoConditions and cryptoconditions don't match up.

This comment has been minimized.

@mellery451

mellery451 Feb 6, 2017

Contributor

good idea - added.

@mellery451

mellery451 Feb 6, 2017

Contributor

good idea - added.

using namespace test::jtx;
Env env {*this};
auto jrr = env.rpc("feature", "CryptoConditions") [jss::result];

This comment has been minimized.

@miguelportilla

miguelportilla Feb 6, 2017

Member

Suggestion:
BEAST_EXPECTS(jrr[jss::status] == jss::success, "status");
or
if (! BEAST_EXPECTS(jrr[jss::status] == jss::success, "status") return;

@miguelportilla

miguelportilla Feb 6, 2017

Member

Suggestion:
BEAST_EXPECTS(jrr[jss::status] == jss::success, "status");
or
if (! BEAST_EXPECTS(jrr[jss::status] == jss::success, "status") return;

// since that is what feature RPC actually checks
env.app().getAmendmentTable().enable(featureCryptoConditions);
auto jrr = env.rpc("feature", "CryptoConditions") [jss::result];

This comment has been minimized.

@miguelportilla

miguelportilla Feb 6, 2017

Member

Suggestion:
BEAST_EXPECTS(jrr[jss::status] == jss::success, "status");
or
if (! BEAST_EXPECTS(jrr[jss::status] == jss::success, "status") return;

@miguelportilla

miguelportilla Feb 6, 2017

Member

Suggestion:
BEAST_EXPECTS(jrr[jss::status] == jss::success, "status");
or
if (! BEAST_EXPECTS(jrr[jss::status] == jss::success, "status") return;

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Feb 8, 2017

Codecov Report

Merging #1988 into develop will increase coverage by 0.15%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #1988      +/-   ##
===========================================
+ Coverage    67.25%   67.41%   +0.15%     
===========================================
  Files          683      683              
  Lines        49231    49231              
===========================================
+ Hits         33111    33188      +77     
+ Misses       16120    16043      -77
Impacted Files Coverage Δ
src/ripple/app/ledger/PendingSaves.h 93.54% <0%> (-6.46%)
src/ripple/net/impl/RPCCall.cpp 37.31% <0%> (+1.99%)
src/ripple/app/misc/impl/AmendmentTable.cpp 92.78% <0%> (+20.19%)
src/ripple/rpc/handlers/Feature1.cpp 92.59% <0%> (+92.59%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b605b3...fd7e23a. Read the comment docs.

codecov-io commented Feb 8, 2017

Codecov Report

Merging #1988 into develop will increase coverage by 0.15%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #1988      +/-   ##
===========================================
+ Coverage    67.25%   67.41%   +0.15%     
===========================================
  Files          683      683              
  Lines        49231    49231              
===========================================
+ Hits         33111    33188      +77     
+ Misses       16120    16043      -77
Impacted Files Coverage Δ
src/ripple/app/ledger/PendingSaves.h 93.54% <0%> (-6.46%)
src/ripple/net/impl/RPCCall.cpp 37.31% <0%> (+1.99%)
src/ripple/app/misc/impl/AmendmentTable.cpp 92.78% <0%> (+20.19%)
src/ripple/rpc/handlers/Feature1.cpp 92.59% <0%> (+92.59%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b605b3...fd7e23a. Read the comment docs.

@mellery451

This comment has been minimized.

Show comment
Hide comment
@mellery451

mellery451 Feb 8, 2017

Contributor

@miguelportilla thanks for the review - I have addressed your comments in 752098f

Contributor

mellery451 commented Feb 8, 2017

@miguelportilla thanks for the review - I have addressed your comments in 752098f

@mellery451 mellery451 added the Passed label Feb 14, 2017

@mellery451 mellery451 removed the Passed label Mar 1, 2017

@mellery451

This comment has been minimized.

Show comment
Hide comment
@mellery451

mellery451 Mar 1, 2017

Contributor

this test involves a validator config and will need to be modified following upcoming validator manifest changes

Contributor

mellery451 commented Mar 1, 2017

this test involves a validator config and will need to be modified following upcoming validator manifest changes

@nbougalis nbougalis added the Passed label Mar 17, 2017

@mellery451 mellery451 removed the Passed label Mar 17, 2017

Add test for feature RPC (RIPD-1391):
Create unit test for feature RPC method. Add client_error field to env
RPC requests to provide information about parsing errors.

@mellery451 mellery451 added the Passed label Mar 21, 2017

@nbougalis nbougalis closed this Apr 1, 2017

@mellery451 mellery451 deleted the mellery451:feature_test branch Apr 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment