fix(tests) improve reliability for plugins suite

* improve Serf error handling when invoking commands
* re-enable lua-resty-upload in OpenResty install
* pattern for datadog request size
* increase size or error logs tailing on HTTP 500
* disable syslog tests (cannot grep /var/log/*** on travis-ci because of
  permission issues)
* increase JWT test reliability by making sure the `exp` field is an
  older timestamp

.ci/setup_env.sh

@@ -32,7 +32,6 @@ if [ ! "$(ls -A $CACHE_DIR)" ]; then
       --with-http_realip_module \
       --with-http_stub_status_module \
       --without-lua_resty_websocket \
-      --without-lua_resty_upload \
       --without-lua_resty_mysql \
       --without-lua_resty_redis \
       --without-http_redis_module \

kong/serf.lua

@@ -37,10 +37,10 @@ function Serf:invoke_signal(signal, args, no_rpc)
   local cmd = string.format("%s %s %s %s", self.config.serf_path, signal, rpc, tostring(args))
   local ok, code, stdout, stderr = pl_utils.executeex(cmd)
   if not ok or code ~= 0 then
-    ngx.log(ngx.ERR, pl_utils.executeex('echo $PATH'))
-    ngx.log(ngx.ERR, "ok: ", ok, " code: ", code, " stdout: ", stdout, " stderr: ", stderr)
+    -- always print the first error line of serf
+    local err = stdout ~= "" and pl_stringx.splitlines(stdout)[1] or stderr
+    return nil, err
   end
-  if not ok or code ~= 0 then return nil, pl_stringx.splitlines(stdout)[1] end -- always print the first error line of serf
 
   return stdout
 end

spec/02-integration/00-helpers/00-helpers_spec.lua

@@ -23,7 +23,7 @@ describe("helpers: assertions and modifiers", function()
   end)
 
   before_each(function()
-    client = helpers.proxy_client(3000)
+    client = helpers.proxy_client(5000)
   end)
   after_each(function()
     if client then client:close() end

spec/02-integration/01-cmd/08-restart_spec.lua

@@ -44,11 +44,13 @@ describe("kong restart", function()
     }
 
     assert(helpers.kong_exec("start --conf "..helpers.test_conf_path, env))
+    ngx.sleep(1)
     local serf_pid = assert(helpers.file.read(helpers.test_conf.serf_pid))
     local nginx_pid = assert(helpers.file.read(helpers.test_conf.nginx_pid))
     local dnsmasq_pid = assert(helpers.file.read(helpers.test_conf.dnsmasq_pid))
 
     assert(helpers.kong_exec("restart --prefix "..helpers.test_conf.prefix, env))
+    ngx.sleep(1)
     assert.is_not.equal(assert(helpers.file.read(helpers.test_conf.nginx_pid)), nginx_pid)
     assert.is_not.equal(assert(helpers.file.read(helpers.test_conf.serf_pid)), serf_pid)
     assert.is_not.equal(assert(helpers.file.read(helpers.test_conf.dnsmasq_pid)), dnsmasq_pid)

spec/03-plugins/005-syslog/01-log_spec.lua

@@ -3,7 +3,7 @@ local utils = require "kong.tools.utils"
 local cjson = require "cjson"
 local pl_stringx = require "pl.stringx"
 
-describe("Plugin: syslog (log)", function()
+describe("#ci Plugin: syslog (log)", function()
   local client, platform
   setup(function()
     assert(helpers.start_kong())

spec/03-plugins/008-datadog/01-log_spec.lua

@@ -64,10 +64,10 @@ describe("Plugin: datadog (log)", function()
     assert.True(ok)
     assert.equal(5, #gauges)
     assert.contains("kong.datadog1_com.request.count:1|c", gauges)
-    --assert.contains("kong.datadog1_com.latency:247|g", gauges) -- latency changes, we only check it exists with length 5
-    assert.contains("kong.datadog1_com.request.size:101|g", gauges)
+    assert.contains("kong.datadog1_com.latency:%d+|g", gauges, true)
+    assert.contains("kong.datadog1_com.request.size:%d+|g", gauges, true)
     assert.contains("kong.datadog1_com.request.status.200:1|c", gauges)
-    assert.contains("kong.datadog1_com.response.size:894|g", gauges)
+    assert.contains("kong.datadog1_com.response.size:%d+|g", gauges, true)
   end)
 
   it("logs only given metrics", function()

spec/03-plugins/06-jwt/01-jwt_parser_spec.lua

@@ -104,7 +104,7 @@ describe("Plugin: jwt (parser)", function()
       assert.same({exp = "must be a number", nbf = "must be a number"}, errors)
     end)
     it("checks the exp claim", function()
-      local token = jwt_parser.encode({exp = os.time()}, "secret")
+      local token = jwt_parser.encode({exp = os.time() - 10}, "secret")
       local jwt = assert(jwt_parser:new(token))
 
       local ok, errors = jwt:verify_registered_claims({"exp"})

spec/helpers.lua

@@ -34,8 +34,8 @@ local dao = DAOFactory(conf)
 -----------------
 local resty_http_proxy_mt = {}
 
--- Case insensitive lookup function, returns the value and the original key. Or if not
--- found nil and the search key
+-- Case insensitive lookup function, returns the value and the original key. Or
+-- if not found nil and the search key
 -- @usage -- sample usage
 -- local test = { SoMeKeY = 10 }
 -- print(lookup(test, "somekey"))  --> 10, "SoMeKeY"
@@ -90,10 +90,11 @@ end
 -- @section http_client
 
 --- Send a http request. Based on https://github.com/pintsized/lua-resty-http.
--- If `opts.body` is a table and "Content-Type" header contains `application/json`,
--- `www-form-urlencoded`, or `multipart/form-data`, then it will automatically encode the body
--- according to the content type.
--- If `opts.query` is a table, a query string will be constructed from it and appended
+-- If `opts.body` is a table and "Content-Type" header contains
+-- `application/json`, `www-form-urlencoded`, or `multipart/form-data`, then it
+-- will automatically encode the body according to the content type.
+-- If `opts.query` is a table, a query string will be constructed from it and
+-- appended
 -- to the request path (assuming none is already present).
 -- @name http_client:send
 -- @param opts table with options. See https://github.com/pintsized/lua-resty-http
@@ -146,7 +147,8 @@ function resty_http_proxy_mt:send(opts)
 
   local res, err = self:request(opts)
   if res then
-    -- wrap the read_body() so it caches the result and can be called multiple times
+    -- wrap the read_body() so it caches the result and can be called multiple
+    -- times
     local reader = res.read_body
     res.read_body = function(self)
       if (not self._cached_body) and (not self._cached_error) then
@@ -169,7 +171,7 @@ function resty_http_proxy_mt:__index(k)
 end
 
 
---- Creates a http client. Based on https://github.com/pintsized/lua-resty-http.
+--- Creates a http client. Based on https://github.com/pintsized/lua-resty-http
 -- @name http_client
 -- @param host hostname to connect to
 -- @param port port to connect to
@@ -212,7 +214,8 @@ end
 -- @section servers
 
 --- Starts a TCP server.
--- Accepts a single connection and then closes, echoing what was received (single read).
+-- Accepts a single connection and then closes, echoing what was received
+-- (single read).
 -- @name tcp_server
 -- @param `port`    The port where the server will be listening to
 -- @return `thread` A thread object
@@ -238,8 +241,10 @@ local function tcp_server(port, ...)
 end
 
 --- Starts a HTTP server.
--- Accepts a single connection and then closes. Sends a 200 ok, 'Connection: close' response.
--- If the request received has path `/delay` then the response will be delayed by 2 seconds.
+-- Accepts a single connection and then closes. Sends a 200 ok, 'Connection:
+-- close' response.
+-- If the request received has path `/delay` then the response will be delayed
+-- by 2 seconds.
 -- @name http_server
 -- @param `port`    The port where the server will be listening to
 -- @return `thread` A thread object
@@ -328,8 +333,9 @@ assert = function(...)
   return old_assert(...)
 end
 
--- tricky part: the assertions below, should not reset the `kong_state` inserted above. Hence
--- we shadow the global assert (patched one) with a local assert (unpatched) to prevent this.
+-- tricky part: the assertions below, should not reset the `kong_state`
+-- inserted above. Hence we shadow the global assert (patched one) with a local
+-- assert (unpatched) to prevent this.
 local assert = old_assert
 
 --- Generic modifier "response".
@@ -341,7 +347,8 @@ local assert = old_assert
 -- local res = assert(client:send { ..your request parameters here ..})
 -- local length = assert.response(res).has.header("Content-Length")
 local function modifier_response(state, arguments, level)
-  assert(arguments.n > 0, "response modifier requires a response object as argument")
+  assert(arguments.n > 0,
+        "response modifier requires a response object as argument")
 
   local res = arguments[1]
 
@@ -360,14 +367,15 @@ luassert:register("modifier", "response", modifier_response)
 -- assertions will operate on the value set.
 -- The request must be inside a 'response' from mockbin.org or httpbin.org
 -- @name request
--- @param response results from `http_client:send` function. The request will be extracted from the response.
+-- @param response results from `http_client:send` function. The request will
+-- be extracted from the response.
 -- @usage
 -- local res = assert(client:send { ..your request parameters here ..})
 -- local length = assert.request(res).has.header("Content-Length")
 local function modifier_request(state, arguments, level)
-  local generic = "The assertion 'request' modifier takes a http response object as "..
-                  "input to decode the json-body returned by httpbin.org/mockbin.org, "..
-                  "to retrieve the proxied request."
+  local generic = "The assertion 'request' modifier takes a http response"
+                .." object as input to decode the json-body returned by"
+                .." httpbin.org/mockbin.org, to retrieve the proxied request."
 
   local res = arguments[1]
 
@@ -378,8 +386,8 @@ local function modifier_request(state, arguments, level)
   body = assert(res:read_body())
   body, err = cjson.decode(body)
 
-  assert(body, "Expected the http response object to have a json encoded body, "..
-               "but decoding gave error '"..tostring(err).."'. "..generic)
+  assert(body, "Expected the http response object to have a json encoded body,"
+             .." but decoding gave error '"..tostring(err).."'. "..generic)
 
   -- check if it is a mockbin request
   if lookup((res.headers or {}),"X-Powered-By") ~= "mockbin" then
@@ -395,8 +403,9 @@ local function modifier_request(state, arguments, level)
 end
 luassert:register("modifier", "request", modifier_request)
 
---- Generic fail assertion. A convenience function for debugging tests, always fails. It will output the
--- values it was called with as a table, with an `n` field to indicate the number of arguments received.
+--- Generic fail assertion. A convenience function for debugging tests, always
+-- fails. It will output the values it was called with as a table, with an `n`
+-- field to indicate the number of arguments received.
 -- @name fail
 -- @param ... any set of parameters to be displayed with the failure
 -- @usage
@@ -420,7 +429,8 @@ luassert:register("assertion", "fail", fail,
 -- @name contains
 -- @param expected The value to search for
 -- @param array The array to search for the value
--- @param pattern (optional) If thruthy, then `expected` is matched as a string pattern
+-- @param pattern (optional) If thruthy, then `expected` is matched as a string
+-- pattern
 -- @return the index at which the value was found
 -- @usage
 -- local arr = { "one", "three" }
@@ -455,15 +465,17 @@ luassert:register("assertion", "contains", contains,
 --- Assertion to check the statuscode of a http response.
 -- @name status
 -- @param expected the expected status code
--- @param response (optional) results from `http_client:send` function, alternatively use `response`.
+-- @param response (optional) results from `http_client:send` function,
+-- alternatively use `response`.
 -- @return the response body as a string
 -- @usage
 -- local res = assert(client:send { .. your request params here .. })
 -- local body = assert.has.status(200, res)             -- or alternativly
 -- local body = assert.response(res).has.status(200)    -- does the same
 local function res_status(state, args)
   assert(not kong_state.kong_request,
-         "Cannot check statuscode against a request object, only against a response object")
+         "Cannot check statuscode against a request object,"
+       .." only against a response object")
 
   local expected = args[1]
   local res = args[2] or kong_state.kong_response
@@ -490,7 +502,7 @@ local function res_status(state, args)
       end
 
       local str_t = pl_stringx.splitlines(str)
-      local first_line = #str_t - math.min(20, #str_t) + 1
+      local first_line = #str_t - math.min(60, #str_t) + 1
       local msg_t = {"\nError logs ("..conf.nginx_err_logs.."):"}
       for i = first_line, #str_t do
         msg_t[#msg_t+1] = str_t[i]
@@ -533,12 +545,13 @@ Body:
 %s]])
 luassert:register("assertion", "status", res_status,
                   "assertion.res_status.negative", "assertion.res_status.positive")
-luassert:register("assertion", "res_status", res_status,     -- TODO: remove this, for now will break too many existing tests
+luassert:register("assertion", "res_status", res_status,
                   "assertion.res_status.negative", "assertion.res_status.positive")
 
 --- Checks and returns a json body of an http response/request. Only checks
--- validity of the json, does not check appropriate headers. Setting the target to check
--- can be done through `request` or `response` (requests are only supported with mockbin.com).
+-- validity of the json, does not check appropriate headers. Setting the target
+-- to check can be done through `request` or `response` (requests are only
+-- supported with mockbin.com).
 -- @name jsonbody
 -- @return the decoded json as a table
 -- @usage
@@ -714,7 +727,8 @@ luassert:register("assertion", "formparam", req_form_param,
 -- @section Shell-helpers
 
 --- Execute a command.
--- Modified version of `pl.utils.executeex()` so the output can directly be used on an assertion.
+-- Modified version of `pl.utils.executeex()` so the output can directly be
+-- used on an assertion.
 -- @name execute
 -- @param ... see penlight documentation
 -- @return ok, stderr, stdout; stdout is only included when the result was ok
@@ -736,7 +750,6 @@ end
 local function kong_exec(cmd, env)
   cmd = cmd or ""
   env = env or {}
-  env.serf_path = conf.serf_path
 
   local env_vars = ""
   for k, v in pairs(env) do