From 1a51cf7272110fd6e36ef037c902119ee933cded Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Thu, 19 Aug 2021 04:56:04 +0000 Subject: [PATCH] Cleanup dependencies for testEvent unit test (#886) Many objects linked to the testEvent unit test were there for legacy reasons and no longer needed. Converting Event::dump() to Packable interface additionally removes all dependency on the Store and StoreEntry API. Vastly simplifying the complexity of the test setup and clarifying (lack of) code coverage by this test. --- src/Makefile.am | 240 ++-------------------------------------- src/event.cc | 30 +++-- src/event.h | 5 +- src/tests/stub_event.cc | 2 +- src/tests/testEvent.cc | 60 +++++----- 5 files changed, 53 insertions(+), 284 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 2dcc867579..48716d9e80 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2869,243 +2869,21 @@ tests_testConfigParser_LDFLAGS = $(LIBADD_DL) check_PROGRAMS += tests/testEvent tests_testEvent_SOURCES = \ - $(DELAY_POOL_SOURCE) \ - $(DNSSOURCE) \ - $(HTCPSOURCE) \ - $(IPC_SOURCE) \ - $(SNMP_SOURCE) \ - $(UNLINKDSOURCE) \ - $(WIN32_SOURCE) \ - AccessLogEntry.cc \ - AuthReg.h \ - BodyPipe.cc \ - tests/stub_CacheDigest.cc \ - CacheDigest.h \ - CachePeer.cc \ - CachePeer.h \ - ClientInfo.h \ - tests/stub_CollapsedForwarding.cc \ - ConfigOption.cc \ - ConfigParser.cc \ - CpuAffinityMap.cc \ - CpuAffinityMap.h \ - CpuAffinitySet.cc \ - CpuAffinitySet.h \ - tests/stub_ETag.cc \ tests/testEvent.cc \ - tests/testEvent.h \ - EventLoop.cc \ - EventLoop.h \ - ExternalACLEntry.cc \ - FadingCounter.cc \ - FileMap.h \ - FwdState.cc \ - FwdState.h \ - HappyConnOpener.cc \ - HappyConnOpener.h \ - HttpBody.cc \ - HttpBody.h \ - tests/stub_HttpControlMsg.cc \ - HttpHdrCc.cc \ - HttpHdrCc.h \ - HttpHdrContRange.cc \ - HttpHdrRange.cc \ - HttpHdrSc.cc \ - HttpHdrScTarget.cc \ - HttpHeader.cc \ - HttpHeader.h \ - HttpHeaderFieldInfo.h \ - HttpHeaderFieldStat.h \ - HttpHeaderTools.cc \ - HttpHeaderTools.h \ - HttpReply.cc \ - HttpRequest.cc \ - tests/stub_HttpUpgradeProtocolAccess.cc \ - IoStats.h \ - tests/stub_IpcIoFile.cc \ - LogTags.cc \ - MasterXaction.cc \ - MasterXaction.h \ - MemBuf.cc \ - MemObject.cc \ - MemStore.cc \ - Notes.cc \ - Notes.h \ - Parsing.cc \ - PeerPoolMgr.cc \ - PeerPoolMgr.h \ - Pipeline.cc \ - Pipeline.h \ - RefreshPattern.h \ - RemovalPolicy.cc \ - RequestFlags.cc \ - RequestFlags.h \ - ResolvedPeers.cc \ - ResolvedPeers.h \ - tests/stub_SBufDetailedStats.cc \ - SquidMath.cc \ - SquidMath.h \ - StatCounters.cc \ - StatCounters.h \ - StatHist.cc \ - StatHist.h \ - StoreFileSystem.cc \ - StoreIOState.cc \ - tests/stub_StoreMeta.cc \ - StoreMetaUnpacker.cc \ - StoreSwapLogData.cc \ - StrList.cc \ - StrList.h \ - String.cc \ - Transients.cc \ - tests/stub_cache_cf.cc \ - cache_cf.h \ - cache_manager.cc \ - tests/stub_carp.cc \ - carp.h \ - cbdata.cc \ - clientStream.cc \ - tests/stub_client_db.cc \ - client_side.cc \ - client_side.h \ - client_side_reply.cc \ - client_side_request.cc \ - debug.cc \ - dlink.cc \ - dlink.h \ - errorpage.cc \ + tests/testEvent.h +nodist_tests_testEvent_SOURCES = \ event.cc \ - external_acl.cc \ - tests/stub_fatal.cc \ - fatal.h \ - fd.cc \ - fd.h \ - fde.cc \ - filemap.cc \ - fqdncache.cc \ - fqdncache.h \ - fs_io.cc \ - fs_io.h \ - tests/stub_gopher.cc \ - gopher.h \ - helper.cc \ - hier_code.h \ - http.cc \ - icp_v2.cc \ - icp_v3.cc \ - int.cc \ - int.h \ - internal.cc \ - internal.h \ - tests/stub_ipc_Forwarder.cc \ - ipcache.cc \ - tests/stub_libauth.cc \ - tests/stub_libauth_acls.cc \ - tests/stub_libdiskio.cc \ - tests/stub_liberror.cc \ - tests/stub_libeui.cc \ + MemBuf.cc \ + tests/stub_cache_manager.cc \ + tests/stub_cbdata.cc \ + tests/stub_debug.cc \ tests/stub_libmem.cc \ - tests/stub_libsecurity.cc \ - tests/stub_libstore.cc \ - tests/stub_main_cc.cc \ - mem_node.cc \ - mime.cc \ - mime.h \ - mime_header.cc \ - mime_header.h \ - multicast.cc \ - multicast.h \ - neighbors.cc \ - neighbors.h \ - pconn.cc \ - peer_digest.cc \ - peer_proxy_negotiate_auth.cc \ - peer_proxy_negotiate_auth.h \ - peer_select.cc \ - peer_sourcehash.cc \ - peer_sourcehash.h \ - peer_userhash.cc \ - peer_userhash.h \ - tests/stub_redirect.cc \ - redirect.h \ - refresh.cc \ - refresh.h \ - repl_modules.h \ - stat.cc \ - stat.h \ - stmem.cc \ - store.cc \ - store_client.cc \ - tests/stub_store_digest.cc \ - store_digest.h \ - store_io.cc \ - store_key_md5.cc \ - store_key_md5.h \ - store_log.cc \ - store_log.h \ - store_rebuild.cc \ - store_rebuild.h \ - tests/stub_store_stats.cc \ - store_swapin.cc \ - store_swapin.h \ - store_swapmeta.cc \ - store_swapout.cc \ - tests/CapturingStoreEntry.h \ - time.cc \ - tools.cc \ - tools.h \ - tests/stub_tunnel.cc \ - tunnel.h \ - urn.cc \ - urn.h \ - tests/stub_wccp2.cc \ - wccp2.h \ - tests/stub_whois.cc \ - whois.h \ - wordlist.cc \ - wordlist.h -nodist_tests_testEvent_SOURCES = \ - $(BUILT_SOURCES) + tests/stub_SBuf.cc \ + tests/stub_time.cc \ + tests/stub_tools.cc tests_testEvent_LDADD = \ - libsquid.la \ - clients/libclients.la \ - servers/libservers.la \ - ftp/libftp.la \ - helper/libhelper.la \ - http/libhttp.la \ - proxyp/libproxyp.la \ - parser/libparser.la \ - ident/libident.la \ - acl/libacls.la \ - acl/libstate.la \ - acl/libapi.la \ - dns/libdns.la \ base/libbase.la \ - ip/libip.la \ - fs/libfs.la \ - anyp/libanyp.la \ - icmp/libicmp.la \ - comm/libcomm.la \ - log/liblog.la \ - format/libformat.la \ - $(REPL_OBJS) \ - $(ADAPTATION_LIBS) \ - $(ESI_LIBS) \ - $(SSL_LIBS) \ - $(top_builddir)/lib/libmisccontainers.la \ - $(top_builddir)/lib/libmiscencoding.la \ - $(top_builddir)/lib/libmiscutil.la \ - ipc/libipc.la \ - mgr/libmgr.la \ - store/libstore.la \ - sbuf/libsbuf.la \ - $(SNMP_LIBS) \ - $(NETTLELIB) \ - $(REGEXLIB) \ - $(SSLLIB) \ - $(KRB5LIBS) \ $(LIBCPPUNIT_LIBS) \ - $(SYSTEMD_LIBS) \ $(COMPAT_LIB) \ $(XTRA_LIBS) tests_testEvent_LDFLAGS = $(LIBADD_DL) diff --git a/src/event.cc b/src/event.cc index bc274e0430..4a4759a557 100644 --- a/src/event.cc +++ b/src/event.cc @@ -271,25 +271,21 @@ EventScheduler::clean() } void -EventScheduler::dump(StoreEntry * sentry) +EventScheduler::dump(Packable *out) { - - ev_entry *e = tasks; - if (last_event_ran) - storeAppendPrintf(sentry, "Last event to run: %s\n\n", last_event_ran); - - storeAppendPrintf(sentry, "%-25s\t%-15s\t%s\t%s\n", - "Operation", - "Next Execution", - "Weight", - "Callback Valid?"); - - while (e != NULL) { - storeAppendPrintf(sentry, "%-25s\t%0.3f sec\t%5d\t %s\n", - e->name, e->when ? e->when - current_dtime : 0, e->weight, - (e->arg && e->cbdata) ? cbdataReferenceValid(e->arg) ? "yes" : "no" : "N/A"); - e = e->next; + out->appendf("Last event to run: %s\n\n", last_event_ran); + + out->appendf("%-25s\t%-15s\t%s\t%s\n", + "Operation", + "Next Execution", + "Weight", + "Callback Valid?"); + + for (auto *e = tasks; e; e = e->next) { + out->appendf("%-25s\t%0.3f sec\t%5d\t %s\n", + e->name, (e->when ? e->when - current_dtime : 0), e->weight, + (e->arg && e->cbdata) ? cbdataReferenceValid(e->arg) ? "yes" : "no" : "N/A"); } } diff --git a/src/event.h b/src/event.h index 425056db77..cf7c061ada 100644 --- a/src/event.h +++ b/src/event.h @@ -10,10 +10,9 @@ #define SQUID_EVENT_H #include "AsyncEngine.h" +#include "base/Packable.h" #include "mem/forward.h" -class StoreEntry; - /* event scheduling facilities - run a callback after a given time period. */ typedef void EVH(void *); @@ -57,7 +56,7 @@ public: /* either EVENT_IDLE or milliseconds remaining until the next event */ int timeRemaining() const; /* cache manager output for the event queue */ - void dump(StoreEntry *); + void dump(Packable *); /* find a scheduled event */ bool find(EVH * func, void * arg); /* schedule a callback function to run in when seconds */ diff --git a/src/tests/stub_event.cc b/src/tests/stub_event.cc index f4c0d79f16..2c64338866 100644 --- a/src/tests/stub_event.cc +++ b/src/tests/stub_event.cc @@ -28,7 +28,7 @@ EventScheduler::~EventScheduler() STUB void EventScheduler::cancel(EVH *, void *) STUB int EventScheduler::timeRemaining() const STUB_RETVAL(1) void EventScheduler::clean() STUB -void EventScheduler::dump(StoreEntry *) STUB +void EventScheduler::dump(Packable *) STUB bool EventScheduler::find(EVH *, void *) STUB_RETVAL(false) void EventScheduler::schedule(const char *, EVH *, void *, double, int, bool) STUB int EventScheduler::checkEvents(int) STUB_RETVAL(-1) diff --git a/src/tests/testEvent.cc b/src/tests/testEvent.cc index dafb67fc2f..888cb0e294 100644 --- a/src/tests/testEvent.cc +++ b/src/tests/testEvent.cc @@ -7,14 +7,10 @@ */ #include "squid.h" - -#include - #include "base/AsyncCallQueue.h" -#include "CapturingStoreEntry.h" #include "event.h" -#include "stat.h" -#include "testEvent.h" +#include "MemBuf.h" +#include "tests/testEvent.h" #include "unitTestMain.h" CPPUNIT_TEST_SUITE_REGISTRATION( testEvent ); @@ -25,7 +21,6 @@ void testEvent::setUp() { Mem::Init(); - statInit(); } /* @@ -37,16 +32,15 @@ testEvent::testCreate() EventScheduler scheduler = EventScheduler(); } -/* Helper for tests - an event which records the number of calls it received. */ - -struct CalledEvent { - CalledEvent() : calls(0) {} - +/// Helper for tests - an event which records the number of calls it received +class CalledEvent +{ +public: static void Handler(void *data) { static_cast(data)->calls++; } - int calls; + int calls = 0; }; /* submit two callbacks, and cancel one, then dispatch and only the other should run. @@ -66,20 +60,21 @@ testEvent::testCancel() CPPUNIT_ASSERT_EQUAL(0, event_to_cancel.calls); } -/* submit two callbacks, and then dump the queue. - */ +// submit two callbacks, and then dump the queue. void testEvent::testDump() { EventScheduler scheduler; CalledEvent event; CalledEvent event2; - CapturingStoreEntry * anEntry = new CapturingStoreEntry(); - String expect = "Last event to run: last event\n" - "\n" - "Operation \tNext Execution \tWeight\tCallback Valid?\n" - "test event \t0.000 sec\t 0\t N/A\n" - "test event2 \t0.000 sec\t 0\t N/A\n"; + const char *expected = "Last event to run: last event\n" + "\n" + "Operation \tNext Execution \tWeight\tCallback Valid?\n" + "test event \t0.000 sec\t 0\t N/A\n" + "test event2 \t0.000 sec\t 0\t N/A\n"; + MemBuf expect; + expect.init(); + expect.append(expected, strlen(expected)); scheduler.schedule("last event", CalledEvent::Handler, &event, 0, 0, false); @@ -88,31 +83,32 @@ testEvent::testDump() AsyncCallQueue::Instance().fire(); scheduler.schedule("test event", CalledEvent::Handler, &event, 0, 0, false); scheduler.schedule("test event2", CalledEvent::Handler, &event2, 0, 0, false); - scheduler.dump(anEntry); + + MemBuf result; + result.init(); + scheduler.dump(&result); /* loop over the strings, showing exactly where they differ (if at all) */ printf("Actual Text:\n"); /* TODO: these should really be just [] lookups, but String doesn't have those here yet. */ - for ( unsigned int i = 0; i < anEntry->_appended_text.size(); ++i) { - CPPUNIT_ASSERT( expect[i] ); - CPPUNIT_ASSERT( anEntry->_appended_text[i] ); + for (unsigned int i = 0; i < result.contentSize(); ++i) { + CPPUNIT_ASSERT(expect.content()[i]); + CPPUNIT_ASSERT(result.content()[i]); /* slight hack to make special chars visible */ - switch (anEntry->_appended_text[i]) { + switch (result.content()[i]) { case '\t': printf("\\t"); break; default: - printf("%c", anEntry->_appended_text[i] ); + printf("%c", result.content()[i]); } /* make this an int comparison, so that we can see the ASCII code at failure */ - CPPUNIT_ASSERT_EQUAL( (int)(expect[i]), (int)anEntry->_appended_text[i] ); + CPPUNIT_ASSERT_EQUAL(int(expect.content()[i]), int(result.content()[i])); } printf("\n"); - CPPUNIT_ASSERT_EQUAL( expect, anEntry->_appended_text); - - /* cleanup */ - delete anEntry; + CPPUNIT_ASSERT_EQUAL(expect.contentSize(), result.contentSize()); + CPPUNIT_ASSERT(strcmp(expect.content(), result.content()) == 0); } /* submit two callbacks, and find the right one. -- 2.39.5