thecodingmachine / graphqlite

Use PHP Attributes/Annotations to declare your GraphQL API
https://graphqlite.thecodingmachine.io
MIT License
554 stars 95 forks source link

Add automatically-verified working examples #668

Closed shish closed 3 months ago

shish commented 4 months ago

Add "no framework" example to examples folder

Having an example as actual code rather than as out-of-context snippets inside a text file means that we can run the code, and ensure that it actually works

Submitting mostly for discussion to see if people like this idea

TODO:

codecov-commenter commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.31%. Comparing base (53f9d49) to head (e3d5d1a). Report is 82 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #668 +/- ## ============================================ - Coverage 95.72% 95.31% -0.41% - Complexity 1773 1809 +36 ============================================ Files 154 171 +17 Lines 4586 4783 +197 ============================================ + Hits 4390 4559 +169 - Misses 196 224 +28 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

oojacoboo commented 4 months ago

@shish I think this is a reasonable solution. However, it's not going to remain accurate unless there are tests for it.

We should probably update the .github/workflows/continuous_integration.yml to include a step that can execute each of the examples and assert the results.

Another alternative would be adding a PHPUnit test that executes the examples.

Edit: Glanced over your todo list - agreed.

Let's prioritize the testing setup, as that'll help with assisting on feedback and contributions.

oojacoboo commented 4 months ago

Converted this to a draft until the TODO items are completed.

shish commented 4 months ago

For reasons I am struggling to figure out, this runs fine on my dev machine but fails in github T_T

cd examples/no-framework
composer install --no-interaction --no-progress --prefer-dist
php -S localhost:8080 &
sleep 3
curl --silent -X POST -H "Content-Type: application/json" \
            -d '{"query":"{ hello(name: \"World\") }"}' \
            http://localhost:8080/graphql

Local output:

{"data":{"hello":"Hello World"}}

Output when run on github:

{"errors":[{"message":"Cannot query field \"hello\" on type \"Query\".","locations":[{"line":1,"column":3}]}]}

Interestingly when I run the published graphqlite@6.x it works and graphqlite@7.x gives the same error as we see on github...

oojacoboo commented 4 months ago

For reasons I am struggling to figure out, this runs fine on my dev machine but fails in github T_T

cd examples/no-framework
composer install --no-interaction --no-progress --prefer-dist
php -S localhost:8080 &
sleep 3
curl --silent -X POST -H "Content-Type: application/json" \
            -d '{"query":"{ hello(name: \"World\") }"}' \
            http://localhost:8080/graphql

Local output:

{"data":{"hello":"Hello World"}}

Output when run on github:

{"errors":[{"message":"Cannot query field \"hello\" on type \"Query\".","locations":[{"line":1,"column":3}]}]}

Interestingly when I run the published graphqlite@6.x it works and graphqlite@7.x gives the same error as we see on github...

Version 7 uses https://github.com/alekitto/class-finder, and master will now look to the Composer autoload file for namespaces, by default (will be 7.1).

shish commented 4 months ago

Any idea what needs to change in the code to make it work? If you download this PR and run the test steps from the github action, does it work for you? Any idea why it works on my laptop but not on github?

FYI, I don't have a "known-good" graphqlite setup that I can compare against, so I'm still not sure if my example code is even supposed to work the way it has been written - is it my code that's broken or something wrong with the framework? Without having a working example that I can compare against, I am debugging blind and I don't even know what direction I'm supposed to be going in 😓

(I'm not actually a graphqlite user, just somebody who found the docs so frustrating that it was easier to write my own schema-generator from scratch -- I would like to deprecate my own and use graphqlite, but I can't do that if I can't figure out why graphqlite isn't working, and it's hard to figure out why it's not working if there isn't a working example to compare against 😅 )

oojacoboo commented 3 months ago

@shish see #679 - might be the issue you're having? Also, it'd be nice if we could merge all these PRs you have together. That'd help with getting this merged.

shish commented 3 months ago

@shish see https://github.com/thecodingmachine/graphqlite/issues/679 - might be the issue you're having?

So that fixed the no-framework example, but it didn't fix the psr-15 example, which is having the same error T_T

Also, it'd be nice if we could merge all these PRs you have together. That'd help with getting this merged.

The "automatically test examples as part of github actions" and "the no-framework example" are now working. Since the psr-15 example is broken for seemingly unrelated examples, I wonder if we could merge this PR as-is, and then rebase the other PR on top of it to try and debug what's wrong with that one?