Implements v2 #178

artem-zakharchenko · 2019-05-28T08:40:32Z

These changes implement the next major version of Gavel per defined spec. They must be merged once the spec lands.

Refs

Closes Deprecate and kill async API #68
Closes Add method validation for requests #158
Closes Do not mix internal exceptions and validation results #163

Changelog

See Gavel spec v2 to know the breaking and non-breaking changes

Roadmap

Remove unused code (utils, tests, feature tests setups)
Decide on uri feature, implement if decided

artem-zakharchenko · 2019-05-28T08:43:08Z

CI requires next gavel-spec release to pass successfully.

honzajavorek · 2019-05-28T12:06:27Z

CI requires next gavel-spec release to pass successfully.

I'm wondering whether it wouldn't help if we had those two in one repo 🤔

artem-zakharchenko · 2019-05-30T11:28:04Z

@honzajavorek it's troublesome that spec lives its own life, yet wasn't that the original intent? So that there could be multiple implementations of gavel?

I would say we need to assess if that's useful, and how many Gavel implementations in other languages we have. If the effort is not worth a trouble, I would vote for moving gavel-spec into gavel package (or vice versa), and treating its test suites as behavior tests.

honzajavorek · 2019-05-30T12:09:31Z

yet wasn't that the original intent? So that there could be multiple implementations of gavel?

Yes, it was. I guess we could solve it for once when we have Dredd monorepo where all these things can live, together and separated at the same time.

how many Gavel implementations in other languages we have

After 6 years of gavel.js existence we still have just gavel.js. The strategy did work out for dredd hooks, but not for gavel. It seems there is no audience to it, people want to use gavel as a core of other tools, not to use it separately. You can see the hopes were different - there's gavel-spec, and there's gavel CLI. I'm not sure how many people use the CLI, TBH, gavel as an end product has never took off.

artem-zakharchenko · 2019-06-04T07:44:15Z

As of 71a02bc, gavel.js is compliant with the next version of gavel-spec (https://github.com/apiaryio/gavel-spec/tree/dcda152fb0f404d25aa90cf656be323502b57640).

Roadmap

Ensure method field is validated as per spec (b575e93)
Ensure uri field is validated as per spec

bin/gavel

artem-zakharchenko · 2019-06-11T08:14:13Z

Updated data_model steps in 7e48433.

Previously, code snippets described in features were compared to the parsed HTTP message (parsing done as a part of world.js cucumber setup, not a part of the library's API). Parsed HTTP message was wrapped in one of the following classes from the library's API:

HttpRequest
ExpectedHttpRequet
HttpResponse
ExpectedHttpResponse

On their own those classes did absolutely nothing to perform the parsing, or affect a provided HTTP message (I omit partial normalization during the validation because this is not what data_model feature asserts).

Since aforementioned classes have been removed from the library's public API, test setup cannot rely on dynamic class instantiation anymore:

const foo = gavel[klass](data)

Also, since gavel[klass] doesn't affect the given payload, there is no need to use it during the assertion. I've removed dynamic class instantiation and compare feature code snippet and parsed HTTP message.

I do, however, find this test useless, as it doesn't involve testing actual library's API, and looks like test for the sake of test. I would vote for removing it, which means removing data_model.feature from gavel-spec as well (not the library's feature and doesn't have to be in the spec).

artem-zakharchenko · 2019-06-17T08:51:17Z

Updated to the latest release of gavel-spec, build passes. Need to ensure clean-up is done, and re-request a code review on this pull request.

honzajavorek · 2019-06-17T13:20:26Z

Regarding #178 (comment), I think the only benefit left is documenting how over-the-wire HTTP messages map to Gavel input. If we have proper docs about how to run Gavel and how to structure its inputs, I think it is okay to remove the feature.

honzajavorek

honzajavorek · 2019-06-17T13:26:12Z

test/cucumber/support/world.js

+    if (string === 'true') return true;
+    if (string === 'false') return false;
+    return !!string;
+  }


I think JSON.parse('false') produces false and JSON.parse('true') produces true. Perhaps we could use it instead, for a simplicity?

How would this work if the string is "abc"?
Calling JSON.parse('abc') will throw, and if the JSON.parse(string) is positioned below the !!string it's rendered useless. Maybe I'm not fully understanding what you meant...

I wanted to reply with more context, but then I got stuck not being able to find toBoolean() used. Where is it being called? I couldn't find toCamelCase() or toPascalCase() either.

test/cucumber/step_definitions/validators_steps.js

lib/units/validateURI.js

artem-zakharchenko · 2019-06-17T13:49:46Z

I agree. Data models bear informative value, but they weren't asserting anything in the public's API. As discussed with @kylef, I also voted to include this models in the documentation for general knowledge, but remove from tests.

artem-zakharchenko · 2019-06-18T08:59:59Z

I wonder, would require('url') resolve in the browser? As any dependency, I would expect url to be bundled in order to be executed, yet being a native nodejs dependency I'm not sure what would happen.

@honzajavorek what do you think about this?

artem-zakharchenko · 2019-06-18T09:01:38Z

lib/units/validateURI.js

-    query: parseQueryString(queryString)
+    pathname,
+    port,
+    hash,


Accounting for previously missing hash in the URI.

BREAKING CHANGE: Gavel no longer exposes "isValidatable" method in its public API

BREAKING CHANGE: Gavel no longer exports "isValid" method from public API. Pleae use "gavel.validate" instead.

BREAKING CHANGE: Calling "gavel.validate" no longer requires the "type" parameter.

BREAKING CHANGE: Gavel no longer exports "HttpRequest" and "HttpResponse" classes. Use "validate" instead.

BREAKING CHANGE: Gavel's call signature of the "validate" function is now the following: validate(expected, real)

BREAKING CHANGE: "gavel.validate()" no longer accepts a callback as an argument. Please use the "validate()" function as follows: const result = gavel.validate(expected, real)

ApiaryBot · 2019-06-18T09:43:49Z

🎉 This PR is included in version 6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

honzajavorek · 2019-06-18T09:55:23Z

I think #178 (comment) is a good point. I'm not sure about the consequences. If relative URLs are an issue my idea was to work around it with prefixing it with example.com just for the sake of parsing, but I see you found solutions already in fcd0ccf

artem-zakharchenko added the in progress label May 30, 2019

artem-zakharchenko force-pushed the v2 branch 3 times, most recently from 3d3a2a3 to 9b8a0a0 Compare May 31, 2019 09:20

honzajavorek mentioned this pull request May 31, 2019

Update gavel-spec to the latest version 🚀 #185

Merged

artem-zakharchenko mentioned this pull request Jun 6, 2019

Gavel 2 specification apiaryio/gavel-spec#44

Merged

21 tasks

honzajavorek reviewed Jun 6, 2019

View reviewed changes

bin/gavel Outdated Show resolved Hide resolved

artem-zakharchenko force-pushed the v2 branch from ac08597 to 98ab5eb Compare June 11, 2019 06:48

artem-zakharchenko force-pushed the v2 branch 2 times, most recently from 81d9dc8 to cbf0b5a Compare June 17, 2019 08:39

artem-zakharchenko mentioned this pull request Jun 17, 2019

Update gavel-spec to the latest version 🚀 #196

Closed

artem-zakharchenko requested a review from honzajavorek June 17, 2019 09:12

honzajavorek approved these changes Jun 17, 2019

View reviewed changes

artem-zakharchenko commented Jun 18, 2019

View reviewed changes

artem-zakharchenko added 6 commits June 18, 2019 11:10

feat: removes "isValidatable" from public API

33dc9b8

BREAKING CHANGE: Gavel no longer exposes "isValidatable" method in its public API

test: removes "isValidatable" test suites

f0960fe

feat: stops using "isValid" internally

807a879

feat: removes "isValid" from public API

969ab5a

BREAKING CHANGE: Gavel no longer exports "isValid" method from public API. Pleae use "gavel.validate" instead.

feat: removes "type" parameter from "validate" call signature

4a4e9a3

BREAKING CHANGE: Calling "gavel.validate" no longer requires the "type" parameter.

feat: removes HttpRequest, HttpResponse, and Validatable

2c36cbb

BREAKING CHANGE: Gavel no longer exports "HttpRequest" and "HttpResponse" classes. Use "validate" instead.

artem-zakharchenko added 21 commits June 18, 2019 11:10

test: aligns test suites for gavel-spec v2

a7a0074

feat: adds "method" field validation

e5c4f72

feat: removes "severity" from error messages

290dec8

refactor: adjusts methods and tests to be compliant with the next spec

de99480

chore: exports gavel directly from lib/

01503a0

feat: swaps the order of "real" and "expected" arguments (validate)

f631116

BREAKING CHANGE: Gavel's call signature of the "validate" function is now the following: validate(expected, real)

feat: removes "callback" argument of "validate"

681b032

BREAKING CHANGE: "gavel.validate()" no longer accepts a callback as an argument. Please use the "validate()" function as follows: const result = gavel.validate(expected, real)

chore: removes unnecessary modules nesting around "validateMessage"

8270f29

test: removes "sets some errors for X" criteria handling (cucumber)

818d1d2

refactor: introduces "otherwise" logic util

4478590

feat: validates "uri" field of HTTP message

f08d0a6

fix: validates relative URI

7344f12

chore: uses "*.test.js" pattern for tests

b8ae32e

chore: moves all tests under "test/" dir

8f0a170

fix: uses new call signature of "validate()" in cli

4748564

feat: validates query parameters (validateURI)

1dfeab4

test: asserts feature snippet and parsed data in "data_model" tests

329759b

chore: updates "gavel-spec" to version 3

459c4e8

fix: improves explanatory comments

7870a99

fix: adjusts namespaces and call signatures of "isValid" utils

c830ff6

fix: uses native "url" module for URI parsing

5bd3333

artem-zakharchenko force-pushed the v2 branch from fcd0ccf to 5bd3333 Compare June 18, 2019 09:12

artem-zakharchenko merged commit 0138366 into master Jun 18, 2019

artem-zakharchenko deleted the v2 branch June 18, 2019 09:41

ApiaryBot added the released label Jun 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implements v2 #178

Implements v2 #178

artem-zakharchenko commented May 28, 2019 •

edited

Loading

artem-zakharchenko commented May 28, 2019

honzajavorek commented May 28, 2019

artem-zakharchenko commented May 30, 2019 •

edited

Loading

honzajavorek commented May 30, 2019

artem-zakharchenko commented Jun 4, 2019 •

edited

Loading

artem-zakharchenko commented Jun 11, 2019 •

edited

Loading

artem-zakharchenko commented Jun 17, 2019

honzajavorek commented Jun 17, 2019

honzajavorek left a comment

honzajavorek Jun 17, 2019

artem-zakharchenko Jun 18, 2019 •

edited

Loading

honzajavorek Jun 18, 2019

artem-zakharchenko commented Jun 17, 2019

artem-zakharchenko commented Jun 18, 2019

artem-zakharchenko Jun 18, 2019

ApiaryBot commented Jun 18, 2019

honzajavorek commented Jun 18, 2019

Implements v2 #178

Implements v2 #178

Conversation

artem-zakharchenko commented May 28, 2019 • edited Loading

Refs

Changelog

Roadmap

artem-zakharchenko commented May 28, 2019

honzajavorek commented May 28, 2019

artem-zakharchenko commented May 30, 2019 • edited Loading

honzajavorek commented May 30, 2019

artem-zakharchenko commented Jun 4, 2019 • edited Loading

Roadmap

artem-zakharchenko commented Jun 11, 2019 • edited Loading

artem-zakharchenko commented Jun 17, 2019

honzajavorek commented Jun 17, 2019

honzajavorek left a comment

Choose a reason for hiding this comment

honzajavorek Jun 17, 2019

Choose a reason for hiding this comment

artem-zakharchenko Jun 18, 2019 • edited Loading

Choose a reason for hiding this comment

honzajavorek Jun 18, 2019

Choose a reason for hiding this comment

artem-zakharchenko commented Jun 17, 2019

artem-zakharchenko commented Jun 18, 2019

artem-zakharchenko Jun 18, 2019

Choose a reason for hiding this comment

ApiaryBot commented Jun 18, 2019

honzajavorek commented Jun 18, 2019

artem-zakharchenko commented May 28, 2019 •

edited

Loading

artem-zakharchenko commented May 30, 2019 •

edited

Loading

artem-zakharchenko commented Jun 4, 2019 •

edited

Loading

artem-zakharchenko commented Jun 11, 2019 •

edited

Loading

artem-zakharchenko Jun 18, 2019 •

edited

Loading