-
Notifications
You must be signed in to change notification settings - Fork 146
Conversation
I'm trying to contribute a fix for https://github.com/neo4j-graphql/neo4j-graphql-js When I am about to contribute to a software library, the first thing I do is to look for software tests and run them. Unfortunately, there is no documentation how to setup tests. What I did: * `npm install` - intuitive * I checked `package.json` - aha, there is `npm run test` and `npm run test-all` * When I run `npm run test-all` all the integration tests fail. Anything missing? Checking CircleCI https://circleci.com/gh/neo4j-graphql/neo4j-graphql-js/3519 reveals I have to `node -r babel-register ./example/apollo-server/movies-middleware.js` * Scrolling down reveals `npm run parse-tck` 😦 * And finally `./node_modules/.bin/nyc ./node_modules/.bin/ava test/*.js test/neo4j-schema/*Test.js` Ok, so first suggestion: I would expect all the above instruction to go into `CONTRIBUTING.md`. Alternatively it should be in `README.md` in a section `Contributing`. I checked the other projects of neo4j here is what I discovered: The following repos of `neo4j` use the exact same `CONTRIBUTING.md`: * https://github.com/neo4j/neo4j/blob/3.5/CONTRIBUTING.md * https://github.com/neo4j/neo4j-java-driver/blob/2.0/CONTRIBUTING.md * https://github.com/neo4j/neo4j-javascript-driver/blob/1.7/CONTRIBUTING.md * https://github.com/neo4j/neo4j-go-driver/blob/1.7/CONTRIBUTING.md So that's why I decided to commit the same file to the repository. I find the link to the neo4j-users slack helpful but the rest of it is pretty generic and does not explain how to setup tests. So where does that information go? Next up: Why are your `npm` scripts different from what you use on CircleCI? For me the build server is the single source of truth. It confuses me that CircleCI uses a different set of tests than your `npm run test-all` for example. Are all the tests supposed to pass, after all? On my machine, plenty of integration tests fail and I'm confused if they might be expected to fail. So, dear contributors, @johnymontana and @#2michaeldgraham how do I setup tests? Cheers, Robert
Codecov Report
@@ Coverage Diff @@
## master #243 +/- ##
=======================================
Coverage 94.47% 94.47%
=======================================
Files 4 4
Lines 326 326
=======================================
Hits 308 308
Misses 18 18 Continue to review full report at Codecov.
|
For the record, here are my specs: $ uname -a
Linux e480 5.0.13-arch1-1-ARCH #1 SMP PREEMPT Sun May 5 18:05:41 UTC 2019 x86_64 GNU/Linux
$ java -version
openjdk version "1.8.0_212"
OpenJDK Runtime Environment (build 1.8.0_212-b01)
OpenJDK 64-Bit Server VM (build 25.212-b01, mixed mode)
$ neo4j version
Error: Could not find or load main class org.neo4j.server.CommunityEntryPoint
# interesting, but https://aur.archlinux.org/packages/neo4j-community/ tells me I'm on version 3.5.3-1
$ node --version
v10.15.1
$ npm --version
6.9.0
$ git rev-parse HEAD
536a3abbfe9939523172cc5f85c06a389c6baf7d |
When I run the commands from circleci (single source of truth):
Terminal output
|
Hey @roschaefer - thanks for pointing out the issues here and I'm sorry for your frustration.
The integration (these are mostly end-to-end tests) tests require a bit more setup, including a locally running Neo4j instance with a specific database loaded and starting a GraphQL server with specific typedefs and middleware loaded. You can see the scripts used to download/start these services here and in the CircleCI config. Finally, the TCK tests are an initial version of compliance tests that we'll have in place across different implementations of Neo4j GraphQL (currently JavaScript and Java). These tests are generated from markdown files and the goal is for the same tests to be run across implementations, ensuring consistency. These are not quite in final form yet and a bit WIP. This is described (although tersely) in the README here.. I agree that this is currently poorly documented and makes it more difficult to contribute to this project so I'll work on simplifying the process and describing the steps to get the full tests running as well as more helpful contributing guidelines. However, since this is a Neo4j Labs project the contributing guide found on other Neo4j projects is not completely relevant, so I'm closing this PR and have opened issue #248 to track adding a comprehensive contributing.md to this project. |
I like that @johnymontana has closed my pull request here: neo4j-graphql#243 I don't find the standard CONTRIBUTING guidelines very helpful either. The only thing that I like to re-use is a hint how to join the neo4j community on Slack. I guess that would be the best option for new contributors to get some guidance. So in this commit I tried to document issues and my workarounds that I discovered already. I hope other people find this useful.
I'm trying to contribute a fix for #239
When I am about to contribute to a software library, the first thing I do is to
look for software tests and run them. Unfortunately, there is no documentation
how to setup tests.
What I did:
npm install
- intuitivepackage.json
- aha, there isnpm run test
andnpm run test-all
npm run test-all
all the integration tests fail. Anythingmissing? Checking CircleCI https://circleci.com/gh/neo4j-graphql/neo4j-graphql-js/3519
reveals I have to
node -r babel-register ./example/apollo-server/movies-middleware.js
npm run parse-tck
😦./node_modules/.bin/nyc ./node_modules/.bin/ava test/*.js test/neo4j-schema/*Test.js
Ok, so first suggestion: I would expect all the above instruction to go into
CONTRIBUTING.md
. Alternatively it should be inREADME.md
in a sectionContributing
. I checked the other projects of neo4j here is what I discovered:The following repos of
neo4j
use the exact sameCONTRIBUTING.md
:So that's why I decided to commit the same file to the repository. I find the
link to the neo4j-users slack helpful but the rest of it is pretty generic and
does not explain how to setup tests. So where does that information go?
Next up: Why are your
npm
scripts different from what you use on CircleCI? Forme the build server is the single source of truth. It confuses me that CircleCI
uses a different set of tests than your
npm run test-all
for example. Are allthe tests supposed to pass, after all? On my machine, plenty of integration
tests fail and I'm confused if they might be expected to fail.
So, dear contributors, @johnymontana and @michaeldgraham how do I setup tests?
Cheers,
Robert