From 1cf7defee88db324e937a0e7c9d247c80a03603b Mon Sep 17 00:00:00 2001 From: Cristian Klein Date: Mon, 26 Oct 2020 12:33:13 +0100 Subject: [PATCH] [auto-clear] Address feedback --- libgnucash/app-utils/test/test-autoclear.cpp | 174 ++++++++++--------- 1 file changed, 90 insertions(+), 84 deletions(-) diff --git a/libgnucash/app-utils/test/test-autoclear.cpp b/libgnucash/app-utils/test/test-autoclear.cpp index 73d9ae173e..780e67bb03 100644 --- a/libgnucash/app-utils/test/test-autoclear.cpp +++ b/libgnucash/app-utils/test/test-autoclear.cpp @@ -31,7 +31,7 @@ extern "C" { #include "gtest/gtest.h" -const gint64 DENOM = 100; //< Denomerator is always 100 for simplicity. +static const int64_t DENOM = 100; //< Denomerator is always 100 for simplicity. struct SplitDatum { const char *memo; @@ -39,100 +39,108 @@ struct SplitDatum { bool cleared; }; -struct TestCase { +struct Tests { gint64 amount; const char *expectedErr; }; -std::vector easySplitData = { - { "Memo 01", - 8234, true }, - { "Memo 02", -156326, true }, - { "Memo 03", - 4500, true }, - { "Memo 04", -694056, true }, - { "Memo 05", - 7358, true }, - { "Memo 06", - 11700, true }, - { "Memo 07", - 20497, true }, - { "Memo 08", - 11900, true }, - { "Memo 09", - 8275, true }, - { "Memo 10", - 58700, true }, - { "Memo 11", +100000, true }, - { "Memo 12", - 13881, true }, - { "Memo 13", - 5000, true }, - { "Memo 14", +200000, true }, - { "Memo 15", - 16800, true }, - { "Memo 16", -152000, true }, - { "Memo 17", +160000, false }, - { "Memo 18", - 63610, false }, - { "Memo 19", - 2702, false }, - { "Memo 20", - 15400, false }, - { "Memo 21", - 3900, false }, - { "Memo 22", - 22042, false }, - { "Memo 23", - 2900, false }, - { "Memo 24", - 10900, false }, - { "Memo 25", - 44400, false }, - { "Memo 26", - 9200, false }, - { "Memo 27", - 7900, false }, - { "Memo 28", - 1990, false }, - { "Memo 29", - 7901, false }, - { "Memo 30", - 61200, false }, -}; - -std::vector easyTestCases = { - { 0, "The selected amount cannot be cleared.", }, - { -869227, "Account is already at Auto-Clear Balance." }, // No splits need to be cleared. - { -869300, "The selected amount cannot be cleared." }, - { -869230, NULL }, - { -963272, NULL }, // All splits need to be cleared. +struct TestCase { + std::vector splits; + std::vector tests; }; -std::vector ambiguousSplitData = { - { "Memo 01", -10, false }, - { "Memo 02", -10, false }, - { "Memo 03", -10, false }, +TestCase easyTestCase = { + .splits = { + { "Memo 01", - 8234, true }, + { "Memo 02", -156326, true }, + { "Memo 03", - 4500, true }, + { "Memo 04", -694056, true }, + { "Memo 05", - 7358, true }, + { "Memo 06", - 11700, true }, + { "Memo 07", - 20497, true }, + { "Memo 08", - 11900, true }, + { "Memo 09", - 8275, true }, + { "Memo 10", - 58700, true }, + { "Memo 11", +100000, true }, + { "Memo 12", - 13881, true }, + { "Memo 13", - 5000, true }, + { "Memo 14", +200000, true }, + { "Memo 15", - 16800, true }, + { "Memo 16", -152000, true }, + { "Memo 17", +160000, false }, + { "Memo 18", - 63610, false }, + { "Memo 19", - 2702, false }, + { "Memo 20", - 15400, false }, + { "Memo 21", - 3900, false }, + { "Memo 22", - 22042, false }, + { "Memo 23", - 2900, false }, + { "Memo 24", - 10900, false }, + { "Memo 25", - 44400, false }, + { "Memo 26", - 9200, false }, + { "Memo 27", - 7900, false }, + { "Memo 28", - 1990, false }, + { "Memo 29", - 7901, false }, + { "Memo 30", - 61200, false }, + }, + .tests = { + { 0, "The selected amount cannot be cleared.", }, + { -869227, "Account is already at Auto-Clear Balance." }, // No splits need to be cleared. + { -869300, "The selected amount cannot be cleared." }, + { -869230, NULL }, + { -963272, NULL }, // All splits need to be cleared. + }, }; -std::vector ambiguousTestCases = { - { -10, "Cannot uniquely clear splits. Found multiple possibilities." }, - { -20, "Cannot uniquely clear splits. Found multiple possibilities." }, - - // Commented out, auto-clear algorithm is not smart enough yet. - //{ -30, NULL }, +TestCase ambiguousTestCase = { + .splits = { + { "Memo 01", -10, false }, + { "Memo 02", -10, false }, + { "Memo 03", -10, false }, + }, + .tests = { + { -10, "Cannot uniquely clear splits. Found multiple possibilities." }, + { -20, "Cannot uniquely clear splits. Found multiple possibilities." }, + + // Forbid auto-clear to be too smart. We expect the user to manually deal + // with such situations. + { -30, "Cannot uniquely clear splits. Found multiple possibilities." }, + }, }; -class AutoClearTest - : public testing::TestWithParam< - std::pair< - std::vector, - std::vector - > - > { +class AutoClearTest : public testing::TestWithParam { +protected: + std::shared_ptr book_; + Account *account_; // owned by book_ + TestCase testCase_; + +public: + AutoClearTest() : + book_(qof_book_new(), qof_book_destroy), + account_(xaccMallocAccount(book_.get())) + { + testCase_ = GetParam(); + + xaccAccountSetName(account_, "Test Account"); + xaccAccountBeginEdit(account_); + for (auto &d : testCase_.splits) { + Split *split = xaccMallocSplit(book_.get()); + xaccSplitSetMemo(split, d.memo); + xaccSplitSetAmount(split, gnc_numeric_create(d.amount, DENOM)); + xaccSplitSetReconcile(split, d.cleared ? CREC : NREC); + xaccSplitSetAccount(split, account_); + + gnc_account_insert_split(account_, split); + } + xaccAccountCommitEdit(account_); + } }; TEST_P(AutoClearTest, DoesAutoClear) { - auto splitData = GetParam().first; - auto testCase = GetParam().second; - - QofBook *book = qof_book_new (); - Account *account = xaccMallocAccount(book); - xaccAccountSetName(account, "Test Account"); - - xaccAccountBeginEdit(account); - for (auto &d : splitData) { - Split *split = xaccMallocSplit(book); - xaccSplitSetMemo(split, d.memo); - xaccSplitSetAmount(split, gnc_numeric_create(d.amount, DENOM)); - xaccSplitSetReconcile(split, d.cleared ? CREC : NREC); - xaccSplitSetAccount(split, account); - - gnc_account_insert_split(account, split); - } - xaccAccountCommitEdit(account); - - for (auto &t : testCase) { + for (auto &t : testCase_.tests) { gnc_numeric amount_to_clear = gnc_numeric_create(t.amount, DENOM); char *err; - GList *splits_to_clear = gnc_account_get_autoclear_splits(account, amount_to_clear, &err); + GList *splits_to_clear = gnc_account_get_autoclear_splits(account_, amount_to_clear, &err); // Actually clear splits for (GList *node = splits_to_clear; node; node = node->next) { @@ -142,20 +150,18 @@ TEST_P(AutoClearTest, DoesAutoClear) { ASSERT_STREQ(err, t.expectedErr); if (t.expectedErr == NULL) { - gnc_numeric c = xaccAccountGetClearedBalance(account); + gnc_numeric c = xaccAccountGetClearedBalance(account_); ASSERT_EQ(c.num, t.amount); ASSERT_EQ(c.denom, DENOM); } } - - qof_book_destroy(book); } INSTANTIATE_TEST_SUITE_P( InstantiationAutoClearTest, AutoClearTest, testing::Values( - std::pair{ easySplitData, easyTestCases }, - std::pair{ ambiguousSplitData, ambiguousTestCases } + easyTestCase, + ambiguousTestCase ) );