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

Separate input and output in doTest functions #4341

Merged
merged 1 commit into from Aug 17, 2017

Conversation

Projects
None yet
5 participants
@pirapira
Member

pirapira commented Aug 16, 2017

Currently, various doTest(json_spirit::mValue&) functions modify the JSON value. Sometimes a filler is given but gradually morphed into a finished test. I discussed this structure with @winsvega and @gumb0. The conclusion was we should separate the input and the output.

This PR just changes the type of doTest so that it takes a const reference and returns a JSON value. The internal workings of each function are not changed yet.

@pirapira

This comment has been minimized.

Show comment
Hide comment
@pirapira

pirapira Aug 16, 2017

Member

The Appveyor test is just being queued for today.

Member

pirapira commented Aug 16, 2017

The Appveyor test is just being queued for today.

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Aug 16, 2017

Codecov Report

Merging #4341 into develop will decrease coverage by 0.02%.
The diff coverage is 82.85%.

@@             Coverage Diff             @@
##           develop    #4341      +/-   ##
===========================================
- Coverage     67.3%   67.28%   -0.03%     
===========================================
  Files          307      307              
  Lines        23666    23671       +5     
===========================================
- Hits         15929    15926       -3     
- Misses        7737     7745       +8
Impacted Files Coverage Δ
test/tools/libtesteth/TestHelper.h 50% <ø> (ø) ⬆️
test/tools/libtesteth/TestOutputHelper.h 100% <ø> (ø) ⬆️
test/tools/jsontests/TransactionTests.cpp 66.66% <100%> (+0.24%) ⬆️
test/tools/jsontests/vm.cpp 63.85% <100%> (+0.1%) ⬆️
test/tools/jsontests/StateTests.cpp 94.31% <100%> (+0.06%) ⬆️
test/tools/libtesteth/TestOutputHelper.cpp 84.78% <100%> (ø) ⬆️
test/tools/libtesteth/TestHelper.cpp 49.26% <40%> (ø) ⬆️
test/tools/jsontests/BlockChainTests.cpp 30.96% <78.57%> (+0.27%) ⬆️
test/tools/fuzzTesting/fuzzHelper.cpp 44.02% <0%> (-2.99%) ⬇️

codecov-io commented Aug 16, 2017

Codecov Report

Merging #4341 into develop will decrease coverage by 0.02%.
The diff coverage is 82.85%.

@@             Coverage Diff             @@
##           develop    #4341      +/-   ##
===========================================
- Coverage     67.3%   67.28%   -0.03%     
===========================================
  Files          307      307              
  Lines        23666    23671       +5     
===========================================
- Hits         15929    15926       -3     
- Misses        7737     7745       +8
Impacted Files Coverage Δ
test/tools/libtesteth/TestHelper.h 50% <ø> (ø) ⬆️
test/tools/libtesteth/TestOutputHelper.h 100% <ø> (ø) ⬆️
test/tools/jsontests/TransactionTests.cpp 66.66% <100%> (+0.24%) ⬆️
test/tools/jsontests/vm.cpp 63.85% <100%> (+0.1%) ⬆️
test/tools/jsontests/StateTests.cpp 94.31% <100%> (+0.06%) ⬆️
test/tools/libtesteth/TestOutputHelper.cpp 84.78% <100%> (ø) ⬆️
test/tools/libtesteth/TestHelper.cpp 49.26% <40%> (ø) ⬆️
test/tools/jsontests/BlockChainTests.cpp 30.96% <78.57%> (+0.27%) ⬆️
test/tools/fuzzTesting/fuzzHelper.cpp 44.02% <0%> (-2.99%) ⬇️

@pirapira pirapira requested review from winsvega and chfast Aug 16, 2017

@chfast

chfast approved these changes Aug 17, 2017

@pirapira

This comment has been minimized.

Show comment
Hide comment
@pirapira

pirapira Aug 17, 2017

Member

In Travis, a build timed out. Restarting.

Member

pirapira commented Aug 17, 2017

In Travis, a build timed out. Restarting.

@winsvega

This comment has been minimized.

Show comment
Hide comment
@winsvega

winsvega Aug 17, 2017

Contributor

on macos?

Contributor

winsvega commented Aug 17, 2017

on macos?

@pirapira

This comment has been minimized.

Show comment
Hide comment
@pirapira

pirapira Aug 17, 2017

Member

Yes.

Member

pirapira commented Aug 17, 2017

Yes.

@pirapira pirapira merged commit b9a841c into develop Aug 17, 2017

4 checks passed

codecov/patch 82.85% of diff hit (target 80%)
Details
codecov/project Absolute coverage decreased by -0.02% but relative coverage increased by +15.54% compared to c27db2a
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@pirapira pirapira deleted the dotest branch Aug 17, 2017

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