Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for interim response handling #147

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ Possible members of a request object:
`false`.
- `response_body` - String to send as the response body from the origin. Defaults to
the test identifier.
- `interim_responses` - An array of interim responses to send before the final response. Each item can be either:
- `[status_code]` - Just a status code (e.g., `[102]`)
- `[status_code, headers_array]` - Status code and headers, where headers_array is an array of `[header_name, header_value]` pairs
- `response_pause` - Integer number of seconds for the server to pause before generating a response.
- `check_body` - Whether to check the response body. Default `true`.
- `expected_type` - One of:
Expand All @@ -98,6 +101,7 @@ Possible members of a request object:
- `header_name_string` representing headers to check that the response on the client does not include.
- `[header_name_string, header_value_string]`: headers to check that the response is either missing, or if they're present, that they do _not_ contain the given value string (evaluated against the whole header value).
- `expected_response_text` - A string to check the response body against on the client.
- `expected_interim_responses` - An array of interim responses expected to be received by the client. Format is the same as `interim_responses`.
- `setup` - Boolean to indicate whether this is a setup request; failures don't mean the actual test failed.
- `setup_tests` - Array of values that indicate whether the specified check is part of setup; failures don't mean the actual test failed. One of: `["expected_type", "expected_method", "expected_status", "expected_response_headers", "expected_response_text", "expected_request_headers"]`

Expand Down
1 change: 1 addition & 0 deletions docker/nginx/nginx.conf
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ server {
proxy_pass http://localhost:8000;
proxy_cache my-cache;
proxy_cache_revalidate on;
proxy_http_version 1.1;
}
}
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"dependencies": {
"liquidjs": "^10.9.2",
"marked": "^15.0.0",
"npm": "^11.0.0"
"npm": "^11.0.0",
"undici": "^7.4.0"
},
"scripts": {
"server": "node test-engine/server/server.mjs",
Expand Down
4 changes: 4 additions & 0 deletions results/apache.json
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,10 @@
"heuristic-delta-60": true,
"heuristic-delta-600": true,
"heuristic-delta-86400": true,
"interim-102": true,
"interim-103": true,
"interim-no-header-reuse": true,
"interim-not-cached": true,
"invalidate-DELETE": true,
"invalidate-DELETE-cl": [
"Assertion",
Expand Down
16 changes: 16 additions & 0 deletions results/caddy.json
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,22 @@
"Assertion",
"Response 2 does not come from cache"
],
"interim-102": [
"Assertion",
"Interim response 1 not received"
],
"interim-103": [
"Assertion",
"Interim response 1 not received"
],
"interim-no-header-reuse": [
"Assertion",
"Interim response 1 not received"
],
"interim-not-cached": [
"Assertion",
"Interim response 1 not received"
],
"invalidate-DELETE": true,
"invalidate-DELETE-cl": [
"Assertion",
Expand Down
4 changes: 4 additions & 0 deletions results/haproxy.json
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,10 @@
"heuristic-delta-60": true,
"heuristic-delta-600": true,
"heuristic-delta-86400": true,
"interim-102": true,
"interim-103": true,
"interim-no-header-reuse": true,
"interim-not-cached": true,
"invalidate-DELETE": true,
"invalidate-DELETE-cl": [
"Assertion",
Expand Down
16 changes: 16 additions & 0 deletions results/nginx.json
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,22 @@
"Assertion",
"Response 2 does not come from cache"
],
"interim-102": [
"Assertion",
"Response 2 does not come from cache"
],
"interim-103": [
"AbortError",
"This operation was aborted"
],
"interim-no-header-reuse": [
"Assertion",
"Response 2 does not come from cache"
],
"interim-not-cached": [
"Assertion",
"Response 2 does not come from cache"
],
"invalidate-DELETE": [
"Assertion",
"Response 3 comes from cache"
Expand Down
4 changes: 4 additions & 0 deletions results/squid.json
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,10 @@
"heuristic-delta-60": true,
"heuristic-delta-600": true,
"heuristic-delta-86400": true,
"interim-102": true,
"interim-103": true,
"interim-no-header-reuse": true,
"interim-not-cached": true,
"invalidate-DELETE": true,
"invalidate-DELETE-cl": true,
"invalidate-DELETE-failed": true,
Expand Down
16 changes: 16 additions & 0 deletions results/trafficserver.json
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,22 @@
"Assertion",
"Response 2 does not come from cache"
],
"interim-102": [
"AbortError",
"This operation was aborted"
],
"interim-103": [
"Assertion",
"Interim response 1 not received"
],
"interim-no-header-reuse": [
"Assertion",
"Interim response 1 not received"
],
"interim-not-cached": [
"Assertion",
"Interim response 1 not received"
],
"invalidate-DELETE": [
"Setup",
"Response 2 status is 403, not 200"
Expand Down
16 changes: 16 additions & 0 deletions results/varnish.json
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,22 @@
"Assertion",
"Response 2 does not come from cache"
],
"interim-102": [
"Setup",
"Response 1 status is 503, not 200"
],
"interim-103": [
"Setup",
"Response 1 status is 503, not 200"
],
"interim-no-header-reuse": [
"Setup",
"Response 1 status is 503, not 200"
],
"interim-not-cached": [
"Setup",
"Response 1 status is 503, not 200"
],
"invalidate-DELETE": [
"Assertion",
"Response 3 comes from cache"
Expand Down
48 changes: 44 additions & 4 deletions test-engine/client/test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export async function makeTest (test) {
const fetchFunctions = []
for (let i = 0; i < requests.length; ++i) {
fetchFunctions.push({
code: idx => {
code: async idx => {
const reqConfig = requests[idx]
const reqNum = idx + 1
const url = clientUtils.makeTestUrl(uuid, reqConfig)
Expand All @@ -32,11 +32,20 @@ export async function makeTest (test) {
controller.abort()
}, config.requestTimeout * 1000)
init.signal = controller.signal

const interimResponses = []
if ('expected_interim_responses' in reqConfig) {
// Dynamic import since undici is only available in Node.js
const undici = await import('undici')
const dispatcher = new undici.Agent().compose(clientUtils.interimResponsesCollectingInterceptor(interimResponses))
init.dispatcher = dispatcher
}

if (test.dump === true) clientUtils.logRequest(url, init, reqNum)
return fetch(url, init)
.then(response => {
responses.push(response)
return checkResponse(test, requests, idx, response)
return checkResponse(test, requests, idx, response, interimResponses)
})
.finally(() => {
clearTimeout(timeout)
Expand Down Expand Up @@ -84,11 +93,11 @@ export async function makeTest (test) {
})
}

function checkResponse (test, requests, idx, response) {
function checkResponse (test, requests, idx, response, interimResponses) {
const reqNum = idx + 1
const reqConfig = requests[idx]
const resNum = parseInt(response.headers.get('Server-Request-Count'))
if (test.dump === true) clientUtils.logResponse(response, reqNum)
if (test.dump === true) clientUtils.logResponse(response, interimResponses, reqNum)

// catch retries
if (response.headers.has('Request-Numbers')) {
Expand Down Expand Up @@ -185,6 +194,37 @@ function checkResponse (test, requests, idx, response) {
}
})
}

// check interim responses
if ('expected_interim_responses' in reqConfig) {
const isSetup = setupCheck(reqConfig, 'expected_interim_responses')

reqConfig.expected_interim_responses.forEach(([statusCode, headers = []], idx) => {
if (interimResponses[idx] == null) {
assert(isSetup, false, `Interim response ${idx + 1} not received`)
} else {
assert(isSetup, interimResponses[idx][0] === statusCode, `Interim response ${idx + 1} status is ${interimResponses[idx][0]}, not ${statusCode}`)

const receivedHeaders = interimResponses[idx][1]
headers.forEach(([header, value]) => {
if (typeof header === 'string') {
assert(isSetup, header in receivedHeaders,
`Interim response ${idx + 1} ${header} header not present.`)
} else if (header.length > 2) {
assert(isSetup, header in receivedHeaders,
`Interim response ${idx + 1} ${header} header not present.`)

const receivedValue = receivedHeaders[header]
assert(isSetup, value === receivedValue,
`Interim response ${idx + 1} header ${header} is ${receivedValue}, should ${value}`)
} else {
console.log('ERROR: Unknown header item in expected_interim_responses', header)
}
})
}
})
}

return response.text().then(makeCheckResponseBody(test, reqConfig, response.status))
}

Expand Down
52 changes: 51 additions & 1 deletion test-engine/client/utils.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,61 @@ export function logRequest (url, init, reqNum) {
console.log('')
}

export function logResponse (response, reqNum) {
export function logResponse (response, interimResponses, reqNum) {
console.log(`${defines.GREEN}=== Client response ${reqNum}${defines.NC}`)
for (const [statusCode, headers] of interimResponses) {
console.log(` HTTP ${statusCode}`)
for (const [key, value] of Object.entries(headers)) {
console.log(` ${key}: ${value}`)
}
console.log('')
}
console.log(` HTTP ${response.status} ${response.statusText}`)
response.headers.forEach((hvalue, hname) => { // for some reason, node-fetch reverses these
console.log(` ${hname}: ${hvalue}`)
})
console.log('')
}

class InterimResponsesCollectingHandler {
#handler
#interimResponses

constructor (handler, interimResponses) {
this.#handler = handler
this.#interimResponses = interimResponses
}

onRequestStart (controller, context) {
this.#handler.onRequestStart?.(controller, context)
}

onRequestUpgrade (controller, statusCode, headers, socket) {
this.#handler.onRequestUpgrade?.(controller, statusCode, headers, socket)
}

onResponseStart (controller, statusCode, headers, statusMessage) {
if (statusCode < 200) this.#interimResponses.push([statusCode, headers])
this.#handler.onResponseStart?.(controller, statusCode, headers, statusMessage)
}

onResponseData (controller, data) {
this.#handler.onResponseData?.(controller, data)
}

onResponseEnd (controller, trailers) {
this.#handler.onResponseEnd?.(controller, trailers)
}

onResponseError (controller, err) {
this.#handler.onResponseError?.(controller, err)
}
}

export function interimResponsesCollectingInterceptor (collectInto) {
return (dispatch) => {
return (opts, handler) => {
return dispatch(opts, new InterimResponsesCollectingHandler(handler, collectInto))
}
}
}
Loading