From b74366268c48779c2d76941ab768958abd5962ae Mon Sep 17 00:00:00 2001 From: Max Golovanov Date: Wed, 9 Oct 2019 01:36:55 -0700 Subject: [PATCH] Fix HTTP server unit test on Mac --- build-tests.sh | 5 ++- lib/system/EventProperties.cpp | 2 +- tests/common/DebugConsole.hpp | 30 +++++++++++++++ tests/common/HttpServer.hpp | 18 +++++---- tests/common/Reactor.cpp | 60 +++++++++++++++++++---------- tests/common/Reactor.hpp | 2 +- tests/common/SocketTools.hpp | 6 +-- tests/functests/.gitignore | 1 + tests/unittests/.gitignore | 1 + tests/unittests/HttpServerTests.cpp | 11 +++++- 10 files changed, 101 insertions(+), 35 deletions(-) create mode 100644 tests/common/DebugConsole.hpp create mode 100644 tests/functests/.gitignore create mode 100644 tests/unittests/.gitignore diff --git a/build-tests.sh b/build-tests.sh index c01a8a7d..1f46ae1a 100755 --- a/build-tests.sh +++ b/build-tests.sh @@ -1,5 +1,8 @@ #!/bin/sh -./build.sh +cd "${0%/*}" +SKU=${1:-release} +echo Building and running $SKU tests... +./build.sh ${SKU} # Fail on test errors set -e cd out diff --git a/lib/system/EventProperties.cpp b/lib/system/EventProperties.cpp index f0c661e5..94379b26 100644 --- a/lib/system/EventProperties.cpp +++ b/lib/system/EventProperties.cpp @@ -91,7 +91,7 @@ namespace ARIASDK_NS_BEGIN { return *this; } - EventProperties::~EventProperties() + EventProperties::~EventProperties() noexcept { delete m_storage; } diff --git a/tests/common/DebugConsole.hpp b/tests/common/DebugConsole.hpp new file mode 100644 index 00000000..dbc1434a --- /dev/null +++ b/tests/common/DebugConsole.hpp @@ -0,0 +1,30 @@ +// Copyright (c) Microsoft. All rights reserved. +#ifndef TESTS_COMMON_DEBUGCONSOLE_HPP_ +#define TESTS_COMMON_DEBUGCONSOLE_HPP_ + +#ifndef __FUNCSIG__ +#define __FUNCSIG__ __FUNCTION__ +#endif + +#if defined(HAVE_CONSOLE_LOG) && !defined(LOG_DEBUG) +/* Log to console if there's no standard log facility defined */ +# include +# ifndef LOG_DEBUG +# define LOG_DEBUG(fmt_, ...) fprintf(stdout, "%s: " fmt_ "\n", __FUNCSIG__ , ## __VA_ARGS__) +# define LOG_TRACE(fmt_, ...) fprintf(stdout, "%s: " fmt_ "\n", __FUNCSIG__ , ## __VA_ARGS__) +# define LOG_INFO(fmt_, ...) fprintf(stdout, "%s: " fmt_ "\n", __FUNCSIG__ , ## __VA_ARGS__) +# define LOG_WARN(fmt_, ...) fprintf(stderr, "%s: " fmt_ "\n", __FUNCSIG__ , ## __VA_ARGS__) +# define LOG_ERROR(fmt_, ...) fprintf(stderr, "%s: " fmt_ "\n", __FUNCSIG__ , ## __VA_ARGS__) +# endif +#endif + +#ifndef LOG_DEBUG +/* Don't log anything if there's no standard log facility defined */ +# define LOG_DEBUG(fmt_, ...) +# define LOG_TRACE(fmt_, ...) +# define LOG_INFO(fmt_, ...) +# define LOG_WARN(fmt_, ...) +# define LOG_ERROR(fmt_, ...) +#endif + +#endif /* TESTS_COMMON_DEBUGCONSOLE_HPP_ */ diff --git a/tests/common/HttpServer.hpp b/tests/common/HttpServer.hpp index 6b5a39f2..39fcc846 100644 --- a/tests/common/HttpServer.hpp +++ b/tests/common/HttpServer.hpp @@ -63,13 +63,13 @@ class HttpServer : private Reactor::Callback }; protected: - bool allowKeepalive; + std::string m_serverHost; + bool allowKeepalive { true }; Reactor m_reactor; std::list m_listeningSockets; std::list> m_handlers; std::map m_connections; size_t m_maxRequestHeadersSize, m_maxRequestContentSize; - std::string m_serverHost; // Map of killed token to kill duration of that token // Ideally we need a map of string --> std::pairsecond; - char buffer[2048]; + char buffer[2048] = { 0 }; int received = socket.recv(buffer, sizeof(buffer)); LOG_TRACE("HttpServer: [%s] received %d", conn.request.client.c_str(), received); if (received <= 0) { @@ -195,6 +197,7 @@ class HttpServer : private Reactor::Callback virtual void onSocketWritable(Socket socket) override { + LOG_TRACE("HttpServer: writing socket fd=0x%x", socket.m_sock); assert(std::find(m_listeningSockets.begin(), m_listeningSockets.end(), socket) == m_listeningSockets.end()); auto connIt = m_connections.find(socket); @@ -210,6 +213,7 @@ class HttpServer : private Reactor::Callback virtual void onSocketClosed(Socket socket) override { + LOG_TRACE("HttpServer: closing socket fd=0x%x", socket.m_sock); assert(std::find(m_listeningSockets.begin(), m_listeningSockets.end(), socket) == m_listeningSockets.end()); auto connIt = m_connections.find(socket); diff --git a/tests/common/Reactor.cpp b/tests/common/Reactor.cpp index a91abbd8..38b0d008 100644 --- a/tests/common/Reactor.cpp +++ b/tests/common/Reactor.cpp @@ -30,9 +30,11 @@ namespace SocketTools { #ifdef TARGET_OS_MAC struct kevent event; bzero(&event, sizeof(event)); - event.ident = socket; - EV_SET(&event, socket, EVFILT_READ, EV_ADD, 0, 0, NULL); - assert(-1 != kevent(kq, &event, 1, NULL, 0, NULL)); + event.ident = socket.m_sock; + EV_SET(&event, event.ident, EVFILT_READ, EV_ADD, 0, 0, NULL); + kevent(kq, &event, 1, NULL, 0, NULL); + EV_SET(&event, event.ident, EVFILT_WRITE, EV_ADD, 0, 0, NULL); + kevent(kq, &event, 1, NULL, 0, NULL); #endif m_sockets.push_back(SocketData()); m_sockets.back().socket = socket; @@ -117,7 +119,13 @@ namespace SocketTools { if (-1 == kevent(kq, &event, 1, NULL, 0, NULL)) { //// Already removed? - // LOG_ERROR("cannot delete fd=%d from kqueue!", event.ident); + LOG_TRACE("cannot delete fd=%d from kqueue!", event.ident); + } + EV_SET(&event, socket, EVFILT_WRITE, EV_DELETE, 0, 0, NULL); + if (-1 == kevent(kq, &event, 1, NULL, 0, NULL)) + { + //// Already removed? + LOG_TRACE("cannot delete fd=%d from kqueue!", event.ident); } #endif m_sockets.erase(it); @@ -153,7 +161,13 @@ namespace SocketTools { if (-1 == kevent(kq, &event, 1, NULL, 0, NULL)) { //// Already removed? - // LOG_ERROR("cannot delete fd=%d from kqueue!", event.ident); + LOG_TRACE("cannot delete fd=%d from kqueue!", event.ident); + } + EV_SET(&event, sd.socket, EVFILT_WRITE, EV_DELETE, 0, 0, NULL); + if (-1 == kevent(kq, &event, 1, NULL, 0, NULL)) + { + //// Already removed? + LOG_TRACE("cannot delete fd=%d from kqueue!", event.ident); } #endif } @@ -255,21 +269,8 @@ namespace SocketTools { Socket socket = it->socket; int flags = it->flags; - // LOG_TRACE("Reactor: Handling socket %d active flags 0x%x (armed 0x%x)", static_cast(socket), event.flags, event.fflags); - - if ((event.flags & EV_EOF)||(event.flags & EV_ERROR)) - { - m_callback.onSocketClosed(socket); - it->flags=Closed; - struct kevent kevt; - EV_SET(&kevt, event.ident, EVFILT_READ, EV_DELETE, 0, 0, NULL); - if (-1 == kevent(kq, &kevt, 1, NULL, 0, NULL)) - { - //// Already removed? - // LOG_ERROR("cannot delete fd=%d from kqueue!", event.ident); - } - continue; - } + LOG_TRACE("Handling socket %d active flags 0x%08x (armed 0x%08x)", + static_cast(socket), event.flags, event.fflags); if (event.filter==EVFILT_READ) { @@ -292,6 +293,25 @@ namespace SocketTools { } continue; } + + if ((event.flags & EV_EOF)||(event.flags & EV_ERROR)) + { + LOG_TRACE("event.filter=%s", "EVFILT_WRITE"); + m_callback.onSocketClosed(socket); + it->flags=Closed; + struct kevent kevt; + EV_SET(&kevt, event.ident, EVFILT_READ, EV_DELETE, 0, 0, NULL); + if (-1 == kevent(kq, &kevt, 1, NULL, 0, NULL)) + { + LOG_TRACE("cannot delete fd=%d from kqueue!", event.ident); + } + EV_SET(&kevt, event.ident, EVFILT_WRITE, EV_DELETE, 0, 0, NULL); + if (-1 == kevent(kq, &kevt, 1, NULL, 0, NULL)) + { + LOG_TRACE("cannot delete fd=%d from kqueue!", event.ident); + } + continue; + } LOG_ERROR("Reactor: unhandled kevent!"); } #endif diff --git a/tests/common/Reactor.hpp b/tests/common/Reactor.hpp index 1d5efdcb..8fbc1222 100644 --- a/tests/common/Reactor.hpp +++ b/tests/common/Reactor.hpp @@ -51,7 +51,7 @@ class Reactor : protected Thread #ifdef TARGET_OS_MAC /* use kqueue on Mac */ #define KQUEUE_SIZE 32 - int kq; + int kq { 0 }; struct kevent m_events[KQUEUE_SIZE]; #endif diff --git a/tests/common/SocketTools.hpp b/tests/common/SocketTools.hpp index d314f096..1d9d48a1 100644 --- a/tests/common/SocketTools.hpp +++ b/tests/common/SocketTools.hpp @@ -213,7 +213,6 @@ class Socket static Type const Invalid = -1; #endif - protected: Type m_sock; public: @@ -310,7 +309,8 @@ class Socket int recv(_Out_cap_(size) void* buffer, unsigned size) { assert(m_sock != Invalid); - return ::recv(m_sock, reinterpret_cast(buffer), size, 0); + int flags = 0; + return ::recv(m_sock, reinterpret_cast(buffer), size, flags); } int send(void const* buffer, unsigned size) @@ -399,7 +399,7 @@ class Thread { private: std::thread m_thread; - volatile bool m_terminate; + volatile bool m_terminate { false }; protected: Thread() diff --git a/tests/functests/.gitignore b/tests/functests/.gitignore new file mode 100644 index 00000000..84ec5bdf --- /dev/null +++ b/tests/functests/.gitignore @@ -0,0 +1 @@ +/.externalToolBuilders/ diff --git a/tests/unittests/.gitignore b/tests/unittests/.gitignore new file mode 100644 index 00000000..84ec5bdf --- /dev/null +++ b/tests/unittests/.gitignore @@ -0,0 +1 @@ +/.externalToolBuilders/ diff --git a/tests/unittests/HttpServerTests.cpp b/tests/unittests/HttpServerTests.cpp index 7ca83bb1..07e7517a 100644 --- a/tests/unittests/HttpServerTests.cpp +++ b/tests/unittests/HttpServerTests.cpp @@ -2,17 +2,24 @@ #ifndef WIN32_LEAN_AND_MEAN #define WIN32_LEAN_AND_MEAN // Exclude rarely-used stuff from Windows headers #endif + +#include + #include "common/Common.hpp" #include "common/HttpServer.hpp" -using namespace testing; +#ifdef HAVE_CONSOLE_LOG +#undef LOG_DEBUG +#include "common/DebugConsole.hpp" +#endif +using namespace testing; class HttpServerTestsBase : public ::testing::Test { protected: HttpServer server; - int port; + int port { 0 }; Socket clientSocket; public: