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

Accept API Elements as input to avoid parsing #600

Open
XVincentX opened this issue Aug 15, 2016 · 8 comments
Open

Accept API Elements as input to avoid parsing #600

XVincentX opened this issue Aug 15, 2016 · 8 comments

Comments

@XVincentX
Copy link
Contributor

XVincentX commented Aug 15, 2016

I'm working on a super secret integration of Dredd. Given I'm already parsing the ADF on my own, it would be really useful to reuse the current AST instead of make the library reparse the document again.

@XVincentX
Copy link
Contributor Author

XVincentX commented Aug 15, 2016

Given I hit a round issue number (600), this one will have top priority on the to do list.

@kylef kylef changed the title Accept AST as input to avoid parsing Accept API Elements as input to avoid parsing Aug 25, 2017
@kylef
Copy link
Member

kylef commented Aug 25, 2017

This should be relatively easy to implement. Some pointers if anyone is interested in contributing a PR (providing the maintainers would accept).

@honzajavorek
Copy link
Contributor

honzajavorek commented Aug 25, 2017

There are two things the Dredd Transactions library does. It parses the API description document and it compiles HTTP transactions from the parse result.

With time I'm filling with doubts whether it's the correct approach. It began with apiaryio/dredd-transactions#27, continued with @netmilk's thoughts about Dredd being used also in other then CLI runtimes (such as browser, see also #704), and it boils down also to this particular issue.

Basically, I think it would be nice to have dredd-core (the closest current representation would be the Dredd class), which takes API Elements (parse result) and configuration as an input. The dredd CLI package (the closest current representation would be the DreddCommand class) or a wrapper for any other environment would be responsible for providing the inputs (parsing CLI arguments, parsing the API description document).

But that would mean parsing is responsibility of a different package then compilation of the HTTP transactions. That would mean current Dredd Transactions should be split. They should do just one "simple" thing. Take API Elements and compile the list of HTTP transactions.

What do you folks think of this? @XVincentX @kylef @michalholasek @netmilk

@michalholasek
Copy link
Contributor

michalholasek commented Aug 25, 2017

Yeees

As @honzajavorek said, I think we could divide Dredd "ecosystem" to at least 5 modules / packages:

dredd-core - test runner
dredd-parser - description document parsing
dredd-transactions - transaction "compiler"
dredd-cli - command line interface
dredd-reporters - base, HTML, nyan, ...

@honzajavorek
Copy link
Contributor

I'd remove most reporters in favor of TAP, actually: #341 (comment) So I'm not sure about the reporters package. I'd do dredd-hooks to contain the whole hook-handling beast. Also, for backward compatibility, I think it makes sense to keep Dredd's CLI as just dredd.

The parsing is just a couple of lines. I don't know if it even deserves it's own package. Honestly, it does three things:

  1. Defaults unrecognizable API description formats to API Blueprint, with a warning (could this be a part of Fury.js, by an option? at the same time - it's quite specific to Dredd and its history)
  2. Makes sure source maps are available - that's something the compilation can assert later (throwing error if source maps are not available in given API Elements) and ideally it wouldn't be needed at all after Canonical transaction paths  #227
  3. Freezes the API Elements tree - that's something the compilation can do itself as a first thing

I think the parsing can be just part of the CLI package. If any other environment needs the parsing, it won't need the 1 and they can just call Fury.js with the source maps option (2).

@michalholasek
Copy link
Contributor

I don't necessarily mean that dredd-core, dredd-parser, dredd-transactions etc. have to be individual repos and/or npm packages, on the contrary - I would like Dredd to be a monorepo.

@honzajavorek
Copy link
Contributor

I would like Dredd to be a monorepo.

That's happening now. I'll perhaps bring attention of @artem-zakharchenko and @kylef to this issue as it documents our past musing on the topic.

Dredd Transactions should eventually become two packages within the monorepo (parsing and compilation), and with some additional decoupling Dredd, or pieces of Dredd, should be then able to consume API Elements directly.

@XVincentX
Copy link
Contributor Author

👋

I'm still getting notifications on these :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants