search

wework / speccy

Well Spectually 🤓 Enforce quality rules on your OpenAPI 3.0.x specifications.
http://speccy.io
MIT License
828 stars 78 forks source link

-o/--output flag outputs an empty file #276

Closed bramdevries closed 5 years ago

bramdevries commented 5 years ago

Detailed description

Recently upgraded to speccy@0.9 and noticed a bug where speccy resolve api.yml -o output.yml would create an empty file instead of writing the contents.

Did not have any issues with this on speccy@0.8

Context

We have a setup where we use both JSON schemas and OpenAPI 3 specifications and need to use resolve to build a "correct" specification.

Possible implementation

The bug was introduced in https://github.com/wework/speccy/commit/f2f6936f2120652ede833a41f4af0ebcd8ab16c0#diff-4955107868977657681b5dd27b54457aR32, by adding the return resolve outside of the fs.writeFile callback the process does not write any content to the file.

Your environment

I tested this both using the NPM module and the docker image on speccy@0.9. Was also able to reproduce this on the latest master with node speccy.js resolve examples/petstore.yaml -o petstore.yaml

pderaaij commented 5 years ago

Thanks for catching this. I'll add test cases for mentioned issues in the upcoming days.

bramdevries commented 5 years ago

I fixed the issue and added an integration test in https://github.com/wework/speccy/pull/277

dlouzan commented 5 years ago

Just a note: my pending MR #270 also solves this issue in a more general way (there's no need to return from a promise, only resolve/reject, and having a single branch exit for each condition makes the code more readable). There were other issues with that part of the code also affecting the stdin support.

The tests added in #277 are very nice though :-)

dlouzan commented 5 years ago

In fact this is clearly a bug, if an error occurs writing the file, it will reject() IFF the verbose flag is active:

if (err && verbose) {
  console.error('Failed to write file: ' + err.message);
  return reject();
}
mdi commented 5 years ago

I'm also running into this issue. Any update on it?

dlouzan commented 5 years ago

@mdi The PR #270 merged yesterday should solve this. Please check the latest master.

prsbisht98 commented 5 years ago

Is it fixed for speccy@0.9?

djtarazona commented 5 years ago

This is available in master. We'll be cutting a release soon.

dlouzan commented 5 years ago

@djtarazona I'd also ask you to take a look at #294 before cutting a release, since it is also related to fixing configuration options setup. Thanks!

kevineaton commented 5 years ago

This is currently blocking my workflow. We have a very large API and want to go with multiple files, but the output is blank. We are migrating from apidoc to Open API 3 and found speccy.io. It seems like a great tool and would be a perfect fit, if we can get the resolver to work. Is there any information we can provide? Is there a timeline for a fix or an easy temporary workaround? Would love to help if it would be helpful.

AbhimanyuG commented 5 years ago

This is available in master. We'll be cutting a release soon.

@kevineaton I switched to master as @djtarazona suggested.

kevineaton commented 5 years ago

Ah, I missed that. Perfect, thanks! Let me know if I can help test anything.

djtarazona commented 5 years ago

This has been fixed in v0.10.1