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

[PR fails] Adding Stricter Type Checks For Input Object Types Fails only on Node8/9 #321

Closed
isocroft opened this issue May 10, 2020 · 6 comments

Comments

@isocroft
Copy link

isocroft commented May 10, 2020

With respect to my recent PR to this awesome project, I had been researching ways to add stricter type checks to a code-first code-generated graphql schema. I found this package and included it in the openapi-to-graphql package to sanitize and validate input types for OpenAPI v3.x.x parameters and requestBody via their OAS schema definitions.

All Tests Pass on Travis CI/CD (NodeJS v10/11):

image

All Tests Pass on Local (NodeJS v10.17.0)

image

As you see above: All tests on my local copy of the project pass (PS: I am running NodeJS v10.17.0) but on the Travis CI/CD it fails specifically on Node 8/9 and passes on Node 10/11/12.

I suspect that the MQTT subscriptions package is the reason why. But I wanted help in debugging the issue.

@isocroft
Copy link
Author

Here is the list of build jobs and which tests pass and which fail:

image

@Alan-Cha
Copy link
Collaborator

@isocroft Thanks for filling this issue! I think you are correct though. I'm fairly sure this problem is originating from the MQTT subscription package. I'll take a look...

@Alan-Cha
Copy link
Collaborator

Node v8 and v9 are both deprecated. I think in an effort to save everyone, we shouldn't look too deeply into these broken tests. I will update the Travis config to reflect this.

@isocroft
Copy link
Author

@Alan-Cha I looked into this some more and I found out that graphql-mqtt-subscriptions package has a dependency : [email protected] but then the package.json specifies a different one: [email protected] and they seem to be clashing.

@Alan-Cha
Copy link
Collaborator

Hmmm but the tests still work with the other Node versions even though the graphql-mqtt-subscriptions is out of sync. Additionally, I couldn't replicate this error using the same Node version as the one used in the test (v8.14.0). We experienced similar problems when we first pulled in the subscription PR #311 (related #297) but I thought we addressed them. Either way, I still think that we should stop testing on v8, v9, and v11 and unless the LTS versions complain, I don't think we should look into these broken tests too deeply.

@isocroft
Copy link
Author

Thanks a lot @Alan-Cha for the help. I will close out this issue since all the tests now pass

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

No branches or pull requests

2 participants