graphql-go icon indicating copy to clipboard operation
graphql-go copied to clipboard

adding fuzz tests

Open jveiga opened this issue 4 years ago • 8 comments

referencing #386 Added Fuzz tests for query.Parse and query.Parse. How to run with gotip and run

gotip test --fuzz=Fuzz ./internal/query
# or
gotip test --fuzz=Fuzz ./internal/schema
# or
gotip test --fuzz=FuzzSchema .

Note that calling trying to call test fuzz with multiple matching tests will fail with

gotip test --fuzz=Fuzz ./internal/...
cannot use -fuzz flag with multiple packages

jveiga avatar May 17 '20 21:05 jveiga

This is a great idea @jveiga ! Now that we have official fuzz support in Go 1.18, would you be willing to update the PR to use that? Of course, we would need to wait for a few weeks for it to be released, but in the meanwhile we can start migrating this PR.

Any thoughts?

agnivade avatar Dec 10 '21 07:12 agnivade

Yes, indeed the fuzz testing is an amazing idea. Looking forward to Go 1.18. It would be ideal if we could generate more sophisticated queries based on a given schema.

pavelnikolov avatar Dec 10 '21 19:12 pavelnikolov

This is a great idea @jveiga ! Now that we have official fuzz support in Go 1.18, would you be willing to update the PR to use that? Of course, we would need to wait for a few weeks for it to be released, but in the meanwhile we can start migrating this PR.

That's a great approach. I would ask then which functions to fuzz.

Yes, indeed the fuzz testing is an amazing idea. Looking forward to Go 1.18. It would be ideal if we could generate more sophisticated queries based on a given schema.

This kind of fuzzing is easier to do with go-fuzz which I can also add an integration with. I will see what kind of features go 1.18 will support and update this branch with findings

jveiga avatar Dec 11 '21 19:12 jveiga

That's a great approach. I would ask then which functions to fuzz.

I think ParseSchema is a good start. We can keep the resolver constant, and keep changing the schemaString. Later, we can even add (*Schema).Exec

agnivade avatar Dec 12 '21 03:12 agnivade

The real value would come from fuzzing the Exec because this is where user input comes during request time.

pavelnikolov avatar Dec 13 '21 13:12 pavelnikolov

updated with graphql_test.FuzzSchemaExec

Feel free to add more methods to the implementation of query

Also, I realised that calling fuzz stores a local folder with regressions like

❯ tree testdata
testdata
└── fuzz
    └── FuzzSchemaExec
        ├── 671f05b72e69d643e6570ec3777c27147cb4ec551090f2d71ce1c4db1010c686
        └── c843983602e04670a35adb41fb79674de6dd362f431acf11c4f8f6fe7a78df84

Should I add these folders to .gitignore?

jveiga avatar Dec 13 '21 20:12 jveiga

@pavelnikolov running random inputs helps finding unexpected bugs, out of bounds errors, logic errors, etc. This PR started when I found an input that sent the parser into an infinite loop. That's one of the benefits of fuzzing these inputs. It is possible to add an initial corpus with F.Add, we can feed it basic query, mutation and schemas to have better inputs.

jveiga avatar Dec 15 '21 16:12 jveiga

I guess we'll have to wait for Go 1.18 to be released early in 2022 to merge this PR

pavelnikolov avatar Dec 15 '21 21:12 pavelnikolov