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

Maintenance of Offer and PayStrand unit tests #3853

Closed
wants to merge 2 commits into from

Conversation

scottschurr
Copy link
Collaborator

High Level Overview of Change

While doing other work I came across two unit tests that needed maintenance.

Context of Change

  1. Sections of Offer_test.cpp had been made unreadable by clang-format. For those only those unreadable sections I turned off clang-format and returned the formatting to what was there before clang-format was applied. These changes are formatting only. No tests were added, removed, or modified.
  2. I discovered an early return in PayStrand_test.cpp which caused a number of the test to be skipped. I removed the early return and then, with help from @seelabs, repaired the tests that had not been executing and were now failing.

Type of Change

  • Bug fix
  • Tests (You added tests for code that already exists)

Only unit tests were affected by this change, so it does not need to be mentioned in the release notes.

@scottschurr scottschurr added the Tech Debt Non-urgent improvements label May 24, 2021
Copy link
Collaborator

@seelabs seelabs left a comment

Choose a reason for hiding this comment

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

One test needs to be fixed (it's my fault it was broken, not yours). I provided a patch. 👍 after that patch is applied.

D{alice, gw, usdC},
B{USD.issue(), xrpIssue()},
XRPS{bob}));
BEAST_EXPECT(ter == temBAD_PATH);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I messed up this test. Here's a patch with the correction: seelabs@4173a17

Copy link
Collaborator

@seelabs seelabs left a comment

Choose a reason for hiding this comment

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

👍 Thanks for noticing and fixing these!

@scottschurr scottschurr added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label May 26, 2021
This was referenced Jun 2, 2021
@scottschurr scottschurr deleted the test-maint branch June 4, 2021 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. Tech Debt Non-urgent improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants