From 13f0e242413da7b0f859fec423054959b5648a61 Mon Sep 17 00:00:00 2001 From: "Dirk-Jan C. Binnema" Date: Thu, 2 Jun 2022 21:02:11 +0300 Subject: [PATCH] lib: improve test coverage Add a bunch of tests --- Makefile.meson | 3 +- lib/message/mu-contact.cc | 27 +++++++++++++ lib/message/mu-message.cc | 20 +++++----- lib/message/mu-message.hh | 34 +++++++++++----- lib/message/test-mu-message.cc | 28 ++++++++++++- lib/mu-contacts-cache.cc | 39 ++++++++++++++++-- lib/mu-query-results.hh | 2 - lib/mu-store.hh | 13 ++++-- lib/tests/test-mu-maildir.cc | 73 ++++++++++++++++++++++++++++++++++ lib/tests/test-mu-store.cc | 32 ++++++++++++++- lib/utils/mu-utils.cc | 3 +- lib/utils/tests/test-utils.cc | 55 ++++++++++++++++--------- 12 files changed, 275 insertions(+), 54 deletions(-) diff --git a/Makefile.meson b/Makefile.meson index a0b338eb..b60ad8ba 100644 --- a/Makefile.meson +++ b/Makefile.meson @@ -88,9 +88,10 @@ coverage: $(COVERAGE_BUILDDIR) $(NINJA) -C $(COVERAGE_BUILDDIR) test lcov --capture --directory . --output-file $(covfile) @lcov --remove $(covfile) '/usr/*' '*guile*' '*thirdparty*' '*/tests/*' '*mime-object*' --output $(covfile) + @lcov --remove $(covfile) '*mu/*' --output $(covfile) @mkdir -p $(COVERAGE_BUILDDIR)/meson-logs/coverage @genhtml $(covfile) --output-directory $(COVERAGE_BUILDDIR)/meson-logs/coverage/ - @echo "coverage report at: file://$(COVERAGE_BUILDDIR)/meson/logs/coverage/index.html" + @echo "coverage report at: file://$(COVERAGE_BUILDDIR)/meson-logs/coverage/index.html" dist: $(BUILDDIR) @cd $(BUILDDIR); $(MESON) dist diff --git a/lib/message/mu-contact.cc b/lib/message/mu-contact.cc index b31f5fd5..db77df42 100644 --- a/lib/message/mu-contact.cc +++ b/lib/message/mu-contact.cc @@ -169,6 +169,30 @@ test_encode() } +static void +test_sender() +{ + Contact c{"aa@example.com", "Anders Ångström", + Contact::Type::Sender, 54321}; + + assert_equal(c.email, "aa@example.com"); + assert_equal(c.name, "Anders Ångström"); + g_assert_false(c.personal); + g_assert_cmpuint(c.frequency,==,1); + g_assert_cmpuint(c.message_date,==,54321); + + g_assert_false(!!c.field_id()); +} + + +static void +test_misc() +{ + g_assert_false(!!contact_type_from_field_id(Field::Id::Subject)); + +} + + int main(int argc, char* argv[]) { @@ -180,6 +204,9 @@ main(int argc, char* argv[]) g_test_add_func("/message/contact/ctor-cleanup", test_ctor_cleanup); g_test_add_func("/message/contact/encode", test_encode); + g_test_add_func("/message/contact/sender", test_sender); + g_test_add_func("/message/contact/misc", test_misc); + return g_test_run(); } #endif /*BUILD_TESTS*/ diff --git a/lib/message/mu-message.cc b/lib/message/mu-message.cc index c7119a34..aba5b891 100644 --- a/lib/message/mu-message.cc +++ b/lib/message/mu-message.cc @@ -124,12 +124,17 @@ Message::Message(const std::string& text, const std::string& path, Message::Options opts): priv_{std::make_unique(opts)} { + if (text.empty()) + throw Error{Error::Code::InvalidArgument, "text must not be empty"}; + if (!path.empty()) { auto xpath{to_string_opt_gchar(g_canonicalize_filename(path.c_str(), {}))}; if (xpath) priv_->doc.add(Field::Id::Path, std::move(xpath.value())); } + priv_->ctime = ::time({}); + priv_->doc.add(Field::Id::Size, static_cast(text.size())); init_gmime(); @@ -727,9 +732,10 @@ fill_document(Message::Private& priv) case Field::Id::XBodyHtml: doc.add(field.id, priv.body_html); break; - /* ignore */ + /* LCOV_EXCL_START */ case Field::Id::_count_: break; + /* LCOV_EXCL_STOP */ } #pragma GCC diagnostic pop @@ -739,27 +745,21 @@ fill_document(Message::Private& priv) Option Message::header(const std::string& header_field) const { - if (!load_mime_message()) - return Nothing; - + load_mime_message(); return priv_->mime_msg->header(header_field); } Option Message::body_text() const { - if (!load_mime_message()) - return {}; - + load_mime_message(); return priv_->body_txt; } Option Message::body_html() const { - if (!load_mime_message()) - return {}; - + load_mime_message(); return priv_->body_html; } diff --git a/lib/message/mu-message.hh b/lib/message/mu-message.hh index dad7026a..e5f8afec 100644 --- a/lib/message/mu-message.hh +++ b/lib/message/mu-message.hh @@ -43,8 +43,8 @@ public: enum struct Options { None = 0, /**< Defaults */ Decrypt = 1 << 0, /**< Attempt to decrypt */ - RetrieveKeys = 1 << 1, /**< Auto-retrieve crypto keys (implies network - * access) */ + RetrieveKeys = 1 << 1, /**< Auto-retrieve crypto keys (implies network + * access) */ }; /** @@ -77,9 +77,13 @@ public: return Ok(Message{path,opts}); } catch (Error& err) { return Err(err); - } catch (...) { - return Err(Mu::Error(Error::Code::Message, "failed to create message")); } + /* LCOV_EXCL_START */ + catch (...) { + return Err(Mu::Error(Error::Code::Message, + "failed to create message from path")); + } + /* LCOV_EXCL_STOP */ /** * Construct a message based on a Xapian::Document @@ -92,13 +96,18 @@ public: return Ok(Message{std::move(doc)}); } catch (Error& err) { return Err(err); - } catch (...) { - return Err(Mu::Error(Error::Code::Message, "failed to create message")); } + /* LCOV_EXCL_START */ + catch (...) { + return Err(Mu::Error(Error::Code::Message, + "failed to create message from document")); + } + /* LCOV_EXCL_STOP */ + /** * Construct a message from a string. This is mostly useful for testing. - + * * @param text message text * @param path path to message - optional; path does not have to exist. * @param opts options @@ -111,10 +120,13 @@ public: return Ok(Message{text, path, opts}); } catch (Error& err) { return Err(err); - } catch (...) { + } + /* LCOV_EXCL_START */ + catch (...) { return Err(Mu::Error(Error::Code::Message, "failed to create message from text")); } + /* LCOV_EXCL_STOP */ /** * DTOR @@ -233,9 +245,11 @@ public: } /** - * get the last change-time (ctime) for this message + * get the last change-time this message. For path/document-based + * messages this corresponds with the ctime of the underlying file; for + * the text-based ones (as used for testing) it is the creation time. * - * @return chnage time or 0 if unknown + * @return last-change time or 0 if unknown */ ::time_t changed() const { return static_cast<::time_t>(document().integer_value(Field::Id::Changed)); diff --git a/lib/message/test-mu-message.cc b/lib/message/test-mu-message.cc index 838a9e7b..8c6d040d 100644 --- a/lib/message/test-mu-message.cc +++ b/lib/message/test-mu-message.cc @@ -65,7 +65,6 @@ goto * instructions[pOp->opcode]; test_message_1, "/home/test/Maildir/inbox/cur/1649279256.107710_1.evergrey:2,S")}; g_assert_true(!!message); - assert_equal(message->path(), "/home/test/Maildir/inbox/cur/1649279256.107710_1.evergrey:2,S"); g_assert_true(message->maildir().empty()); @@ -99,6 +98,10 @@ goto * instructions[pOp->opcode]; g_assert_true(message->priority() == Priority::Low); g_assert_cmpuint(message->size(),==,::strlen(test_message_1)); + /* text-based message use time({}) as their changed-time */ + g_assert_cmpuint(message->changed() - ::time({}), >=, 0); + g_assert_cmpuint(message->changed() - ::time({}), <=, 2); + g_assert_true(message->references().empty()); assert_equal(message->subject(), @@ -177,7 +180,7 @@ World! auto message{Message::make_from_text(msg_text)}; g_assert_true(!!message); - + g_assert_true(message->has_mime_message()); g_assert_true(message->path().empty()); g_assert_true(message->bcc().empty()); @@ -202,6 +205,10 @@ World! g_assert_true(message->priority() == Priority::Normal); g_assert_cmpuint(message->size(),==,::strlen(msg_text)); + /* text-based message use time({}) as their changed-time */ + g_assert_cmpuint(message->changed() - ::time({}), >=, 0); + g_assert_cmpuint(message->changed() - ::time({}), <=, 2); + assert_equal(message->subject(), "ättächmeñts"); const auto cache_path{message->cache_path()}; @@ -565,6 +572,20 @@ Moi, } +static void +test_message_fail () +{ + { + const auto msg = Message::make_from_path("/root/non-existent-path-12345"); + g_assert_false(!!msg); + } + + { + const auto msg = Message::make_from_text("", ""); + g_assert_false(!!msg); + } +} + int main(int argc, char* argv[]) { @@ -582,6 +603,9 @@ main(int argc, char* argv[]) test_message_multipart_mixed_rfc822); g_test_add_func("/message/message/detect-attachment", test_message_detect_attachment); + g_test_add_func("/message/message/fail", + test_message_fail); + return g_test_run(); } diff --git a/lib/mu-contacts-cache.cc b/lib/mu-contacts-cache.cc index 366d0451..24556aba 100644 --- a/lib/mu-contacts-cache.cc +++ b/lib/mu-contacts-cache.cc @@ -308,9 +308,6 @@ ContactsCache::for_each(const EachContactFunc& each_contact) const { std::lock_guard l_{priv_->mtx_}; - if (!each_contact) - return; // nothing to do - // first sort them for 'rank' ContactSet sorted; for (const auto& item : priv_->contacts_) @@ -409,6 +406,39 @@ test_mu_contacts_cache_personal() } +static void +test_mu_contacts_cache_foreach() +{ + Mu::ContactsCache ccache(""); + ccache.add(Mu::Contact{"a@example.com", "a", 123, true, 1000, 0}); + ccache.add(Mu::Contact{"b@example.com", "b", 456, true, 1000, 0}); + + { + size_t n{}; + g_assert_false(ccache.empty()); + g_assert_cmpuint(ccache.size(),==,2); + ccache.for_each([&](auto&& contact) { ++n; return false; }); + g_assert_cmpuint(n,==,1); + } + + { + size_t n{}; + g_assert_false(ccache.empty()); + g_assert_cmpuint(ccache.size(),==,2); + ccache.for_each([&](auto&& contact) { ++n; return true; }); + g_assert_cmpuint(n,==,2); + } + + { + size_t n{}; + ccache.clear(); + g_assert_true(ccache.empty()); + g_assert_cmpuint(ccache.size(),==,0); + ccache.for_each([&](auto&& contact) { ++n; return true; }); + g_assert_cmpuint(n,==,0); + } +} + static void @@ -418,6 +448,7 @@ test_mu_contacts_cache_sort() std::string str; if (g_test_verbose()) g_print("contacts-cache:\n"); + ccache.for_each([&](auto&& contact) { if (g_test_verbose()) g_print("\t- %s\n", contact.display_name().c_str()); @@ -427,7 +458,6 @@ test_mu_contacts_cache_sort() return str; }; - const auto now{std::time({})}; // "first" means more relevant @@ -473,6 +503,7 @@ main(int argc, char* argv[]) g_test_add_func("/lib/contacts-cache/base", test_mu_contacts_cache_base); g_test_add_func("/lib/contacts-cache/personal", test_mu_contacts_cache_personal); + g_test_add_func("/lib/contacts-cache/for-each", test_mu_contacts_cache_foreach); g_test_add_func("/lib/contacts-cache/sort", test_mu_contacts_cache_sort); g_log_set_handler( diff --git a/lib/mu-query-results.hh b/lib/mu-query-results.hh index d66ee2c5..c2168329 100644 --- a/lib/mu-query-results.hh +++ b/lib/mu-query-results.hh @@ -383,7 +383,6 @@ public: * * @return iterator */ - iterator begin() { return QueryResultsIterator(mset_.begin(), query_matches_); } const iterator begin() const { return QueryResultsIterator(mset_.begin(), query_matches_); } /** @@ -391,7 +390,6 @@ public: * * @return iterator */ - iterator end() { return QueryResultsIterator(mset_.end(), query_matches_); } const_iterator end() const { return QueryResultsIterator(mset_.end(), query_matches_); } /** diff --git a/lib/mu-store.hh b/lib/mu-store.hh index 2f5e92ca..809dda4b 100644 --- a/lib/mu-store.hh +++ b/lib/mu-store.hh @@ -70,9 +70,13 @@ public: } catch (const Mu::Error& me) { return Err(me); - } catch (...) { + } + /* LCOV_EXCL_START */ + catch (...) { return Err(Error::Code::Internal, "failed to create store"); } + /* LCOV_EXCL_STOP */ + struct Config { size_t max_message_size{}; @@ -99,9 +103,12 @@ public: } catch (const Mu::Error& me) { return Err(me); - } catch (...) { - return Err(Error::Code::Internal, "failed to create store"); } + /* LCOV_EXCL_START */ + catch (...) { + return Err(Error::Code::Internal, "failed to create new store"); + } + /* LCOV_EXCL_STOP */ /** * Move CTOR diff --git a/lib/tests/test-mu-maildir.cc b/lib/tests/test-mu-maildir.cc index 904c312c..08bccb97 100644 --- a/lib/tests/test-mu-maildir.cc +++ b/lib/tests/test-mu-maildir.cc @@ -278,6 +278,77 @@ test_determine_target_ok(void) } + +static void +test_determine_target_fail(void) +{ + struct TestCase { + std::string old_path; + std::string root_maildir; + std::string target_maildir; + Flags new_flags; + bool new_name; + std::string expected; + }; + const std::vector testcases = { + TestCase{ /* fail: no absolute path */ + "../foo/Maildir/test/cur/123456:2,FR-not-absolute", + "/home/foo/Maildir", + {}, + Flags::Seen | Flags::Passed, + false, + "/home/foo/Maildir/test/cur/123456:2,PS" + }, + + TestCase{ /* fail: no absolute root */ + "/home/foo/Maildir/test/cur/123456:2,FR", + "../foo/Maildir-not-absolute", + {}, + Flags::New, + false, + "/home/foo/Maildir/test/new/123456" + }, + + TestCase{ /* fail: maildir must start with '/' */ + "/home/foo/Maildir/test/cur/123456", + "/home/foo/Maildir", + "mymaildirwithoutslash", + Flags::Seen | Flags::Flagged, + false, + "/home/foo/Maildir/test/cur/123456:2,FS" + }, + + TestCase{ /* fail: path must be below maildir */ + "/home/foo/Maildir/test/cur/123456:2,FR", + "/home/bar/Maildir", + "/test2", + Flags::Flagged | Flags::Replied, + false, + "/home/foo/Maildir/test2/cur/123456:2,FR" + }, + TestCase{ /* fail: New cannot be combined */ + "/home/foo/Maildir/test/new/123456", + "/home/foo/Maildir", + {}, + Flags::New | Flags::Replied, + false, + "/home/foo/Maildir/test/cur/123456:2," + }, + }; + + for (auto&& testcase: testcases) { + const auto res = maildir_determine_target( + testcase.old_path, + testcase.root_maildir, + testcase.target_maildir, + testcase.new_flags, + testcase.new_name); + g_assert_false(!!res); + } +} + + + static void test_maildir_get_new_path_01(void) { @@ -428,6 +499,8 @@ main(int argc, char* argv[]) g_test_add_func("/mu-maildir/mu-maildir-determine-target-ok", test_determine_target_ok); + g_test_add_func("/mu-maildir/mu-maildir-determine-target-fail", + test_determine_target_fail); // /* get/set flags */ g_test_add_func("/mu-maildir/mu-maildir-get-new-path-01", test_maildir_get_new_path_01); diff --git a/lib/tests/test-mu-store.cc b/lib/tests/test-mu-store.cc index b64e5f34..3e281d92 100644 --- a/lib/tests/test-mu-store.cc +++ b/lib/tests/test-mu-store.cc @@ -139,6 +139,12 @@ goto * instructions[pOp->opcode]; assert_valid_result(docid); g_assert_cmpuint(store->size(),==, 1); + /* ensure 'update' dtrt, i.e., nothing. */ + const auto docid2 = store->update_message(*message, *docid); + assert_valid_result(docid2); + g_assert_cmpuint(store->size(),==, 1); + g_assert_cmpuint(*docid,==,*docid2); + auto msg2{store->find_message(*docid)}; g_assert_true(!!msg2); assert_equal(message->path(), msg2->path()); @@ -228,6 +234,11 @@ World! // for (auto&& term = msg2->document().xapian_document().termlist_begin(); // term != msg2->document().xapian_document().termlist_end(); ++term) // g_message(">>> %s", (*term).c_str()); + + const auto stats{store->statistics()}; + g_assert_cmpuint(stats.size,==,store->size()); + g_assert_cmpuint(stats.last_index,==,0); + g_assert_cmpuint(stats.last_change,>=,::time({})); } @@ -319,6 +330,23 @@ Yes, that would be excellent. } +static void +test_store_fail() +{ + { + const auto store = Store::make("/root/non-existent-path/12345"); + g_assert_false(!!store); + } + + { + const auto store = Store::make_new("/../../root/non-existent-path/12345", + "/../../root/non-existent-path/54321", + {}, {}); + g_assert_false(!!store); + + } +} + int main(int argc, char* argv[]) @@ -331,8 +359,8 @@ main(int argc, char* argv[]) test_message_mailing_list); g_test_add_func("/store/message/attachments", test_message_attachments); - g_test_add_func("/store/index/move", - test_index_move); + g_test_add_func("/store/index/move", test_index_move); + g_test_add_func("/store/index/fail", test_store_fail); if (!g_test_verbose()) g_log_set_handler( diff --git a/lib/utils/mu-utils.cc b/lib/utils/mu-utils.cc index 0e2ac7ca..e1c820f3 100644 --- a/lib/utils/mu-utils.cc +++ b/lib/utils/mu-utils.cc @@ -531,7 +531,8 @@ Mu::parse_size(const std::string& val, bool is_first) if (val.empty()) return is_first ? 0 : std::numeric_limits::max(); - rx = g_regex_new("(\\d+)(b|k|kb|m|mb|g|gb)?", G_REGEX_CASELESS, (GRegexMatchFlags)0, NULL); + rx = g_regex_new("^(\\d+)(b|k|kb|m|mb|g|gb)?$", + G_REGEX_CASELESS, (GRegexMatchFlags)0, NULL); minfo = NULL; if (g_regex_match(rx, val.c_str(), (GRegexMatchFlags)0, &minfo)) { diff --git a/lib/utils/tests/test-utils.cc b/lib/utils/tests/test-utils.cc index e6f721ee..c09671d2 100644 --- a/lib/utils/tests/test-utils.cc +++ b/lib/utils/tests/test-utils.cc @@ -65,7 +65,7 @@ test_date_basic() } g_setenv("TZ", hki, TRUE); - constexpr std::array, 9> cases = {{ + constexpr std::array, 13> cases = {{ {"2015-09-18T09:10:23", true, 1442556623}, {"1972-12-14T09:10:23", true, 93165023}, {"1854-11-18T17:10:23", true, 0}, @@ -73,6 +73,12 @@ test_date_basic() {"2000-02-31T09:10:23", true, 951861599}, {"2000-02-29T23:59:59", true, 951861599}, + {"20220602", true, 1654117200}, + {"20220605", false, 1654462799}, + + {"202206", true, 1654030800}, + {"202206", false, 1656622799}, + {"2016", true, 1451599200}, {"2016", false, 1483221599}, @@ -94,44 +100,55 @@ test_date_basic() static void test_date_ymwdhMs(void) { - struct { - std::string expr; - long diff; - int tolerance; - } tests[] = {{"3h", 3 * 60 * 60, 1}, - {"21d", 21 * 24 * 60 * 60, 3600 + 1}, - {"2w", 2 * 7 * 24 * 60 * 60, 3600 + 1}, + struct testcase { + std::string expr; + int64_t diff; + int tolerance; + }; - {"2y", 2 * 365 * 24 * 60 * 60, 24 * 3600 + 1}, - {"3m", 3 * 30 * 24 * 60 * 60, 3 * 24 * 3600 + 1}}; + std::array cases = {{ + {"7s", 7, 1}, + {"3M", 3 * 60, 1}, + {"3h", 3 * 60 * 60, 1}, + {"21d", 21 * 24 * 60 * 60, 3600 + 1}, + {"2w", 2 * 7 * 24 * 60 * 60, 3600 + 1}, + {"2y", 2 * 365 * 24 * 60 * 60, 24 * 3600 + 1}, + {"3m", 3 * 30 * 24 * 60 * 60, 3 * 24 * 3600 + 1} + }}; - for (auto i = 0; i != G_N_ELEMENTS(tests); ++i) { - const auto diff = ::time({}) - - parse_date_time(tests[i].expr, true).value_or(-1); + for (auto&& tcase: cases) { + const auto date = parse_date_time(tcase.expr, true); + g_assert_true(date); + const auto diff = ::time({}) - *date; if (g_test_verbose()) - std::cerr << tests[i].expr << ' ' << diff << ' ' << tests[i].diff - << std::endl; + std::cerr << tcase.expr << ' ' << diff << ' ' << tcase.diff << '\n'; - g_assert_true(tests[i].diff - diff <= tests[i].tolerance); + g_assert_true(tcase.diff - diff <= tcase.tolerance); } - //g_assert_true(strtol(Mu::date_to_time_t_string("-1y", true).c_str(), NULL, 10) == 0); + // note: perhaps it'd be nice if we'd detect this error; + // currently we're being rather tolerant + // g_assert_false(!!parse_date_time("25q", false)); } static void test_parse_size() { - constexpr std::array, 5> cases = {{ + constexpr std::array, 6> cases = {{ { "456", false, 456 }, { "", false, G_MAXINT64 }, { "", true, 0 }, { "2K", false, 2048 }, - { "2M", true, 2097152 } + { "2M", true, 2097152 }, + { "5G", true, 5368709120 } }}; for(auto&& test: cases) { g_assert_cmpint(parse_size(std::get<0>(test), std::get<1>(test)) .value_or(-1), ==, std::get<2>(test)); } + + g_assert_false(!!parse_size("-1", true)); + g_assert_false(!!parse_size("scoobydoobydoo", false)); } static void