artem-zakharchenko
commented
5 years ago Thanks once more for sharing these insights. Giving my feedback below.
Hooks
All hooks are provided with the real transaction(s) to modify
What implications do you see if we would allow instead to return the next transaction from a hook?
Hooks API
I completely support Hooks API improvements you've mentioned.
Here's how the current API looks like:
const hooks = require('hooks');
hooks.before('Articles > Publish an article', (transaction) => {
transaction.request.headers.Authorization = 'Basic: YWxhZGRpbjpvcGVuc2VzYW1l';
});Get rid of a hacky require('hooks') and let hooks interface be explicitly imported (from Dredd, or a dedicated package), or implicitly injected.
Since hooks can be written in different languages, I don't have a sufficient level of expertise to suggest a proper dependency injection pattern. However, I think it would be less confusing to remove the require call and execute a hook within a certain context. Just like any other testing framework does with injection of their before/after/etc. hooks. This also brings the surface of injection "closer" to Dredd, making it less troublesome for the end users to maintain.
// If this is not installed I'm getting confused.
// IDE may complain module not found.
// What if there is an actual package called "hooks"?
const hooks = require('hooks')
hooks.before()
hooks.afterAll()// No explicit import, but I'm aware this is a hook file
// and I've worked with other testing frameworks, so
// I understand these hook functions are injected for me.
before()
afterAll()Such hook functions are easier to type as well, as we need to ship a type definition module and the end users may simply include it to state those functions are known:
// tsconfig.json
{
"compilerOptions": {
// What type definitions to incliude
"types": ["dredd/hooks"]
},
// Affected files
"include": "hooks.js",
}Typing something like
require('hooks')would mean an introduction of type definitions for a module, which may conflict if such module actually exists.
Change how scenarios may be referenced in the API description document.
- Allow wildcard:
hooks.before('User > *', appendAuthenticationHeader)hooks.after('* > User', arbitraryHookFunc)
I am not sure if it's currently possible to declare something like beforeAll hook with Dredd to trigger a certain logic before a group of scenarios. Generally, I'm open to any kind of API reogranization at this point.
Other
The HTML reporter should render the same locally as the Apiary reporter and apiary.io should share the same (React) components for this
This effectively means re-creating the Tests page from Apiary and shipping it as a part of reporter.
- How would we be able to import the UI components (are they open-sourced)?
- Who will maintain this implementation? There is a build pipeline needed for such reporter (React app) to be built. It sounds like having reporters in separate packages may be a viable option.
describe API in TypeScript annotations
I would absolutely love to do that.
Tests
The changes you've described above affect if not all test suites. I often find the cost of adjusting test suites unjustified and taking more time than actual development. Do you consider to write tests from scratch (TDD, BDD) for this proposal? I know it looks like a monstrous effort at first, but from my practice I believe we would save much more time writing them (properly) from scratch than trying to adjust (and inevitably refactor) existing tests.
Dredd is tested on CircleCI for speed and usability, together with AppVeyor for verifying Windows compatibility.
Perhaps by the time we get to this state CircleCI will already support testing on Windows machines.
Diagram (flow)
- If
TransactionRunnershould accept any recorded traffic as an input that means its input signature must be the same for such recorded traffic and for the list of transactions emitted from theTransactionCompiler. I think we should make a recorded traffic an input toTransactionCompilerso the flow would look like this:
-- traffic --> TransactionCompiler -- transactions --> TransactionRunnerThis makes their responsibilities properly scoped:
- TransactionCompiler: turns input into a list of transactions
- TransactionRunner: runs given transactions. No extra operations, this should be as plain as possible.
I now see there is a dotted arrow pointing from HAR to "Parsing". But there is still a primary error pointing to the TransactionRunner, so I thought to clarify this.
Overall, if we adopt a monorepo pattern together with a clear vision of responsibilities and restrictions of each module, we can start developing this incrementally at a good rate.
I can play around with type definitions for those modules, and brainstorm proper responsibilities of each.
honzajavorek
mathieulaude
Dredd reads the API description document, executes HTTP requests it finds, and validates the HTTP responses it receives.
I think Dredd's architecture should get more modular, decoupled. This is my idea on how it should look like:
Direction
dredd serverfor testing server,dredd clientfor testing clients (aka client-testing mock server: see this),dredd trafficfor testing arbitrary previously recorded trafficSupport
(dredd api) OR (dredd openapi) OR (dredd oas)Architecture
dredd-transactions/parse), the input interface for the Dredd core is API Elementsdredd-transactions/compile).harGardening