Skip to content
This repository has been archived by the owner on Nov 8, 2024. It is now read-only.

Commit

Permalink
test: integrates new implementation into old test suits
Browse files Browse the repository at this point in the history
  • Loading branch information
artem-zakharchenko committed May 16, 2019
1 parent 5220522 commit 334f9ad
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const { assert } = require('chai');
const validateRequest = require('../../validateRequest');
const validateElement = require('../../validateElement');

describe('validateRequest', () => {
describe('validateElement', () => {
describe('with matching requests', () => {
const request = {
method: 'POST',
Expand All @@ -10,7 +10,7 @@ describe('validateRequest', () => {
},
body: '{ "foo": "bar" }'
};
const res = validateRequest(request, request);
const res = validateElement(request, request);

This comment has been minimized.

Copy link
@honzajavorek

honzajavorek May 16, 2019

Contributor

What is Element in this context? If you're looking for a name for "either HTTP request or HTTP response", then it's "HTTP message"

This comment has been minimized.

Copy link
@artem-zakharchenko

artem-zakharchenko May 16, 2019

Author Contributor

Thanks, will use the message instead. Vocabulary improved.

This comment has been minimized.

Copy link
@artem-zakharchenko

artem-zakharchenko May 16, 2019

Author Contributor

Would this reflect the intention of both arguments being the same type (req/res)?

I'm trying to find a proper term that would reflect the validation of two HTTP messages of the same type. My main concern is the follows:

type HttpMessage = 'request' | 'response'
type validateMessage = (real: HttpMessage, expected: HttpMessage)

If my assumption is correct, then that won't suit the purpose of the function.

This comment has been minimized.

Copy link
@honzajavorek

honzajavorek May 16, 2019

Contributor

I think there's no easy way to specify the thing needs to be always of the same type. I'd keep it simple, having httpMessage there and document the constraint in the JS Doc comment.

This comment has been minimized.

Copy link
@honzajavorek

honzajavorek May 16, 2019

Contributor

Also I think while it is okay to use just "request" and "response" in Gavel as in context of the whole code it is clear what those are, it is a good idea to be more verbose in case of the message and have it as "http message", because message is too generic word and could collide with, e.g., error message.

This comment has been minimized.

Copy link
@honzajavorek

honzajavorek May 16, 2019

Contributor

(that last comment can be shortened to 👍 on naming the thing 😄 )


it('returns validation result object', () => {
assert.isObject(res);
Expand Down Expand Up @@ -66,7 +66,7 @@ describe('validateRequest', () => {
});

describe('with non-matching requests', () => {
const res = validateRequest(
const res = validateElement(
{
method: 'POST',
headers: {
Expand All @@ -86,14 +86,17 @@ describe('validateRequest', () => {
});

it('contains all validatable keys', () => {
assert.hasAllKeys(res, ['headers', 'body']);
// Note that "headers" are not present because
// Gavel demands a validatable key to be present
// in both real and expected elements.
assert.hasAllKeys(res, ['body']);
});

describe('method', () => {
it.skip('compares methods');
});

describe('headers', () => {
describe.skip('headers', () => {
it('has no validator set', () => {
assert.propertyVal(res.headers, 'validator', null);
});
Expand Down Expand Up @@ -165,4 +168,8 @@ and expected data media type
});
});
});

describe('with matching responses', () => {
it.skip('add');
});
});
23 changes: 23 additions & 0 deletions lib/api/validate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const normalize = require('./units/normalize');
const validateElement = require('./validateElement');

function validate(real, expected, type, callback) {
const normalizedReal = normalize(real);
const normalizedExpected = normalize(expected);

if (type !== 'request' && type !== 'response') {
throw new Error(
`Can't validate: expected transaction "type" to be "request" or "response", but got: ${type}.`
);
}

const results = validateElement(normalizedReal, normalizedExpected);

// TODO Propagate the error.
callback(null, {
version: '2',
...results
});
}

module.exports = validate;
27 changes: 27 additions & 0 deletions lib/api/validateElement.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
const isset = require('../utils/isset');
const validateStatusCode = require('./units/validateStatusCode');
const validateHeaders = require('./units/validateHeaders');
const { validateBody } = require('./units/validateBody');

function validateElement(real, expected) {

This comment has been minimized.

Copy link
@honzajavorek

honzajavorek May 16, 2019

Contributor

actually, why this isn't directly inside validate()? 🤔

This comment has been minimized.

Copy link
@honzajavorek

honzajavorek May 16, 2019

Contributor

oh, I see, it's because validate has the callback and stuff...

This comment has been minimized.

Copy link
@artem-zakharchenko

artem-zakharchenko May 16, 2019

Author Contributor

validate is a legacy public API and its call signature is overcomplicated. To validate the given HTTTP messages pair we don't need type or callback. In the future I'd like to deprecate the current validate and perform validation using this function. Then we can rename it to validate :)

This comment has been minimized.

Copy link
@honzajavorek

honzajavorek May 16, 2019

Contributor

perfect

const results = {};

if (real.statusCode) {
results.statusCode = validateStatusCode(real, expected);
}

if (real.headers && expected.headers) {
results.headers = validateHeaders(real, expected);
}

if (
isset(real.body) &&

This comment has been minimized.

Copy link
@honzajavorek

honzajavorek May 16, 2019

Contributor

this just reminded me my PHP times 😅 (like, around 2010) https://www.php.net/manual/en/function.isset.php

This comment has been minimized.

Copy link
@artem-zakharchenko

artem-zakharchenko May 16, 2019

Author Contributor

Damn it, years of PHP development can't let me go 😄
I still find this util helpful to check for existence without having false negatives.

This comment has been minimized.

Copy link
@honzajavorek

honzajavorek May 16, 2019

Contributor

isn't something == null enough? (note the double equal sign, catches both null and undefined)

(isset(expected.body) || isset(expected.bodySchema))
) {
results.body = validateBody(real, expected);
}

return results;
}

module.exports = validateElement;
11 changes: 0 additions & 11 deletions lib/api/validateRequest.js

This file was deleted.

3 changes: 2 additions & 1 deletion lib/gavel.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
const { HttpRequest, ExpectedHttpRequest } = require('./model/http-request');
const { HttpResponse, ExpectedHttpResponse } = require('./model/http-response');

const { validate, isValid, isValidatable } = require('./validate');
const { isValid, isValidatable } = require('./validate');
const validate = require('./api/validate');

module.exports = {
HttpRequest,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
},
"scripts": {
"lint": "eslint lib/**/*.js test/**/*.js",
"test": "npm run test:server && npm run test:browser && npm run test:features",
"test": "npm run test:new && npm run test:server && npm run test:browser && npm run test:features",
"test:new": "mocha \"lib/api/test/**/*.test.js\"",
"test:server": "mocha \"test/unit/**/*-test.js\"",
"test:server:coverage": "nyc npm run test:server",
Expand Down
2 changes: 1 addition & 1 deletion test/unit/validate-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { assert } = require('chai');
const clone = require('clone');
const { validate, isValid, isValidatable } = require('../../lib/validate');
const { validate, isValid, isValidatable } = require('../../lib/gavel');

describe('Gavel proxies to functions with callbacks', () => {
// Examples from README.md
Expand Down

0 comments on commit 334f9ad

Please sign in to comment.