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

Enhance AccountTx unit test #2977

Closed
wants to merge 1 commit into from

Conversation

scottschurr
Copy link
Collaborator

While working on tickets I found that the account_tx RPC test did not examine all possible returned transaction types. So I'm adding that test.

Additionally, it seems that there are enough unit tests exercising Checks that it is worth moving jtx interfaces for check handling into a central location.

@ripplelabs-jenkins
Copy link
Collaborator

ripplelabs-jenkins commented Jun 19, 2019

Jenkins Build Summary

Built from this commit

Built at 20190725 - 00:29:02

Test Results

Build Type Log Result Status
msvc.Debug logfile 1178 cases, 0 failed, t: 572s PASS ✅
msvc.Debug,
NINJA_BUILD=true
logfile 1178 cases, 0 failed, t: 600s PASS ✅
gcc.Release
-Dassert=ON,
MANUAL_TESTS=true
logfile 915 cases, 0 failed, t: 4m56s PASS ✅
docs,
TARGET=docs
logfile 1 cases, 0 failed, t: 0m1s PASS ✅
msvc.Debug
-Dunity=OFF
logfile 1177 cases, 0 failed, t: 1093s PASS ✅
clang.Debug logfile 1178 cases, 0 failed, t: 2m56s PASS ✅
gcc.Debug
-Dcoverage=ON,
TARGET=coverage_report,
SKIP_TESTS=true
logfile 1178 cases, 0 failed, t: 16m54s PASS ✅
clang.Debug
-Dunity=OFF
logfile 1177 cases, 0 failed, t: 1m13s PASS ✅
msvc.Release logfile 1178 cases, 0 failed, t: 403s PASS ✅
gcc.Debug
-Dunity=OFF
logfile 1177 cases, 0 failed, t: 0m39s PASS ✅
gcc.Debug logfile 1178 cases, 0 failed, t: 2m59s PASS ✅
clang.Release
-Dassert=ON
logfile 1178 cases, 0 failed, t: 5m6s PASS ✅
gcc.Release
-Dassert=ON
logfile 1178 cases, 0 failed, t: 5m2s PASS ✅
gcc.Debug
-Dstatic=OFF
logfile 1178 cases, 0 failed, t: 2m53s PASS ✅
gcc.Debug
-Dstatic=OFF -DBUILD_SHARED_LIBS=ON
logfile 1178 cases, 0 failed, t: 2m55s PASS ✅
gcc.Debug,
NINJA_BUILD=true
logfile 1178 cases, 0 failed, t: 2m46s PASS ✅
clang.Debug
-Dunity=OFF -Dsan=address,
PARALLEL_TESTS=false,
DEBUGGER=false
logfile 1177 cases, 0 failed, t: 11m25s PASS ✅
clang.Debug
-Dunity=OFF -Dsan=undefined,
PARALLEL_TESTS=false
logfile 1177 cases, 0 failed, t: 11m3s PASS ✅

Copy link
Contributor

@mellery451 mellery451 left a comment

Choose a reason for hiding this comment

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

👍

{ 20, jss::Payment, {jss::AccountRoot}, {}, {jss::AccountRoot}},
};

BEAST_EXPECT (std::extent<decltype (sanity)>::value ==
Copy link
Contributor

Choose a reason for hiding this comment

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

L340 sort of overlaps with this check...I think I like this one better since it verifies against the sanity data

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. Would you prefer that I remove the check on line 340? Or is it okay to leave it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm weakly in favor or removing L340, but I'm fine either way.

@scottschurr
Copy link
Collaborator Author

Rebased to 1.3.0-b6.

@scottschurr scottschurr added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Jul 24, 2019
@scottschurr
Copy link
Collaborator Author

Squashed and rebased to 1.3.0.

@nbougalis nbougalis mentioned this pull request Aug 5, 2019
@scottschurr scottschurr deleted the account-tx-test branch November 19, 2019 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants