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

Automated Testing #122

Open
lastmjs opened this issue Jul 6, 2021 · 0 comments
Open

Automated Testing #122

lastmjs opened this issue Jul 6, 2021 · 0 comments

Comments

@lastmjs
Copy link
Contributor

lastmjs commented Jul 6, 2021

  • Check to make sure that create, update, and delete selection sets have the same capabilities as read selection sets
  • Remember to only turn on database clearing with an explicit sudograph setting...it's so dangerous that maybe we should put other safeguards in place as well
  • Give each test a specific schema to ensure we test important combinations...I think this will be a good solution until random schemas are generated, and might actually always be necessary in addition to randomly generated schemas.
  • I might need to get tokio to compile and run within wasm: Support Webassembly tokio-rs/tokio#1597
  • The emulator might allow me to run tons of tests quickly https://forum.dfinity.org/t/how-to-solve-canister-stack-overflow-problem-and-change-canister-cycel-limit/4940/9
  • Allow end-user developers to be able to plug their schema into the tests to run tests directly on their schema
  • We might need to make testing the automatically generated ids a stand-alone test, and use proptest generated ids everywhere else. This should hopefully allow us to shrink appropriately and potentially get rid of issues where we have to execute a mutation before generating other results
    • Ideally the arbitraries only return queries and mutations that can all be executed at the test level...this should make it easier to integrate into the canister hopefully
  • To get the tests to run really fast, I can have a few canister methods exposed for testing. I can use the Rust testing infrastructure to do initial http requests into the canisters, but from there the canisters can take over...this will not do full integration tests, which I should probably still allow for, but it will test the logic of sudograph directly, just based on the GraphQL query and mutation resolvers. This is probably where we should do most of our testing, because I imagine we could get 100,000s of iterations in a relatively short amount of time, orders of magnitude more than what I am able to achieve right now. This is probably the next thing I should do after the read tests are functioning...because I would love to have 1,000,000s of tests run on this thing for hours at a time...oh wait, will I run into the cycle limit? I really hope I will be able to configure that and turn it off for my tests
  • Consider, for now, creating one static schema per test...just create different schema files and point each test at them
  • The GraphQL variables do not seem to honor nullable vs non-nullable, I seem to be seeing null passed for UpdateRelationManyInput
  • Consider splitting up create and update strategies
  • In the middle of refactoring the blob arbitrary
  • Consider using fallible iterators https://docs.rs/fallible-iterator/0.2.0/fallible_iterator/index.html
  • Make sure to clear the database on every run, the blob append tests...well wait
  • Create schema arbitrary
    • Generate arbitrary schemas and test them for compilation
    • Be careful with one-to-one relations, in that case only one side can be non-nullable...also self-referencing cannot be non-nullable for a one
  • Test create
    • Use cargo scripts instead of npm for binary optimization
    • incorporate agent-rs
    • Write simple one-off test with agent-rs
    • Figure out property library to use
  • Test read
    • Make sure to test basic reading right after creating an object, and make sure to read all of the fields to ensure they are what they should be...it seemed that my update, create, etc tests were missing this, but this might really be the job of the read tests
      • lib.rs line 310, inside the conversion to JSON
    • It seems we might want to test search, limit, offset, order independently, and then all together. We also want to test all of them at arbitrary levels within the selection set on relation manies. I am not sure if the arbitrary selection set should be done independently or as part of each test...I think part of each test sounds good. But we can start by testing just the top level, and then hopefully make the test recursive somehow
    • This will probably be the most complicated
    • Test search
    • Test limit
    • Test offset
    • Test order
    • Test all of the above in selection sets on many relations as well
  • Test update
    • Weird issues with one-to-one relations are not being found by the tests, but I am finding them in the playground. Make the tests more robust
    • Same rules on disconnecting from a relation many to a non-nullable relation one, that must be a runtime error
    • Possible bug...we cannot allow updating a relation one if that would violate the nullabilityness...if you have a relation one to one, and one side is nullable, think about the appropriate thing to do in that situation. You cannot leave the other side empty, so I think it might need to prevent the update
      • Basically, if the nullable relation one has another side to it, then it cannot be updated or created, ever. The other side will always be in control of that. If it just has one side, then it can be updated and created. Unless both sides are nullable, in that case I think you can create or update them at will
      • I think the solution is to only allow a nullable relation one to be set once...if it has another side to it. All updates after the first would happen from the non-nullable other side. You could set it to null again by updating the non-nullable side. But you cannot set it to null directly (what should happen to the other side?) and you cannot set it to another id (what would happen to the other side?), so I believe you just simply cannot set it. Unfortunately this probably has to be a runtime error
      • I think we might want to test making sure an error is returned...if the user tries to update a nullable relation one that is not currently null, then that needs to be a runtime error. See if we can test the runtime error
      • My update tests are not finding this right now, which is concerning
    • Test disconnect
    • Use the correct update input types
    • I am starting to think the append on bytes should be its own test
    • I also think possibly that disconnect should be its own test
    • So far three major types of tests seem appropriate...general updates, append for blob, and disconnect on relation one and relation many
    • Allow any kind of field omitting, not just nullable types. Everything but the id can be omitted possibly
    • I need to handle blobs specially
    • Test the blob append functionality as well, right now I am only testing replace
    • Variables are required, but then how are we testing nullable values?
    • Add an explicit check when connecting, to make sure that the disconnected value is now unassociated...I ran into that issue in the past
      • I have this working if the original field is a relation one...and it will check if its opposing field is a relation one or a relation many. Now I just need to get this to work if the original field is a relation many. The problem is getting an original field that is a relation many...we need to work around any possible recursion issues. This should be possible if we limit the relation many from being created on anything but the first iteration...we could pass down some number indicating the level
    • Should we also test multiple relations, updating multiple times? Right now we always start with an empty multiple relation, and then we simply add one item into it
  • Test delete
    • This should be very similar to update. I will start with an original object, then I will generate an ArbitraryResult that deletes it. Then I will generate one or more ArbitraryResults that read ensuring the object was deleted, and ensuring any relations have removed the deleted object
    • Deletion needs to delete the other side of the relation
    • I think we will have similar rules with non-nullable relations when deleting...see if we can use the same logic in sudodb to do the delete as when we disconnect with an update
    • Write the delete tests first, make sure they fail. Then fix the functionality. Hopefully it will be easy if I can just use sudodb's update functionality to delete the other side of the relation. Basically it's a delete on one side, and then an update to the other side
    • When deleting, we need to go delete the other side of relations as well...otherwise the data will be corrupted
    • seems like deletion needs some cleaning up...should deletion cascade by default?...if you delete one side of the relation, the other side needs to be deleted as well
  • Refactoring to do after good tests in place
    • Use names in the proc macro that no one else would ever use...maybe prefix them with _sudograph or something
    • Get rid of all warnings
    • Remove all mutations
    • Perfect declarativeness
    • Handle all TODO
  • Consider using proptest for async tests, proptest! macro doesn't play nice with tokio::test proptest-rs/proptest#179 there might be some workarounds if I don't use async/await but use the runtime directly
  • Try to resolve: Possible issue with serializing int to async_graphql::Number async-graphql/async-graphql#565
    • Until the above is resolved, the messy serde_json_values_are_equal will have to exist
  • This might be a key to understanding proptest custom arbitraries: https://translate.google.com/translate?hl=en&sl=ja&u=https://qiita.com/legokichi/items/2c3fdcbf84d959668a0f&prev=search&pto=aue
  • consider adding a very simple way to clear the entire database between tests...should not be too difficult, which is pretty neat
  • Update the documentation to explain that custom resolvers are not yet possible, too many issues with Candid
  • Consider multi-threading the tests? They won't be atomic then. It would be interesting for the canister instance to have many simultaneous tests running though
  • Refactor tests to use Result and Option appropriately, get rid of unwrap in inappropriate places
  • Think deeply about the determinism of the tests...because the id is generated automatically on creates, we might have some non-deterministic behavior that is messing with the shrinking
  • Consider having some portion of the tests run through the dfx interface, and some portion actually run within the canister...this would be quite a lot faster...my tests are just taking a long time, which I think is fine for maybe 100 or so iterations to really test things. But it would be great to be able to run 100,000s or 1,000,000s of iterations directly on the graphql interface itself
  • Don't forget to test bytes in the selection set
  • There is currently no way to disconnect with a many to many relationship...we should add this functionality and write tests for it\
  • It seems possible some ids generated by proptest could be identical, such as the empty string. We might want to make sure they are always of sufficient length to hopefully be unique...though testing all strings is nice. I think I have ran into this problem, so I am not sure what to do about it...maybe clearing the database on each iteration will solve the issue

Bugs found

  1. JSON scalars were completely broken for candid serialization
  2. Updating the nullable side of a one-to-one relationship should not be allowed...basically we need to implement foreign key constraints
  3. Leaving out input did not allow object type information to be initialized in the database (relation one?)
  4. Leaving out input did not allow object type information to be initialized in the database for relation many
  5. Made me rethink throwing an error if the offset it too big, now gracefully returns empty (I think that is the better choice)
  6. or was not working correctly, if there is nothing in the or then I think it should return true, not false
  7. ors were being or'd within the field input thing, we needed to get rid of the flat_map
  8. read_input was not returning empty read inputs when relation one had no values inside
  9. Searching for null relations doesn't work properly
  10. I forgot to add startsWith and endsWith to the search within selection sets in Sudograph generate
  11. Search within relation sets was pretty broken...no null values were allowed

Improvements

  • Relation many arbitrary should create a variable number of relations
  • Should we also test multiple relations, updating multiple times? Right now we always start with an empty multiple relation, and then we simply add one item into it
  • Test self-referencing relations
  • Clear the database between tests?
  • Really ensure shrinking is working perfectly
  • do we also want to test that not putting a value in doesn't change the value?
    • for example, we want to make sure that not putting a value in the input doesn't set it to null for some reason
  • Test multiple mutations or queries at a time
  • Do a better test of the append functionality
  • Test many to many updates, ensure that removing from one side removes you from the other side...you know, this might already be covered...
  • Allow any kind of field omitting, not just nullable types. Everything but the id can be omitted possibly
  • So far three major types of tests seem appropriate...general updates, append for blob, and disconnect on relation one and relation many
  • I am starting to think the append on bytes should be its own test
  • The create and update tests might want to do explicit queries after the mutations to ensure that the state changes were effectual...for example, perhaps the update call reverts for some reason, we might still get the correct result saying that the change has occured, but because we did the mutation and the read within the same update call, we aren't entirely sure that the state has changed. This would happen if you accidentally did a mutation through a query call for example
  • proptest::collection::vec should be used for the create and update tests instead of the silly way I am doing it now, the way I am currently doing it is much more complicated
  • Do I actually check deletion behavior when a relation many nullable is actually set to null? (I am pretty sure this is being tested, if the relation many is null on the deleted object side, then the object would be deleted successfully and there would be nothing to check. If the object deleted has a relation and the other side is a nullable relation many...
  • Read tests with very high levels of relations could be interesting...the read test failed with 5 levels on horses, it created one of the largest GraphQL queries I have ever seen. I am not sure why it failed, but it seemed unreasonably large
  • Clean up all code
  • Use idiomatic Rust
  • Get shrinking to work
  • Make tests orders of magnitude faster (best chance is to probably run the tests inside of the emulator)
  • Create combination test for limit, offset, order, and search
  • Create read test for blob
  • Add cross-relational search tests
  • the offset and limit tests are so similar that they should really use generics and closures to reuse most of their code
  • Abstract out the limit functionality to be used for offset, order, and search. I think the functionality is substantially similar and we should only have to implement a few select pieces, I think mostly in the expected value area
  • Get rid of all 'static references if possible, I might be able to just add a 'static generic parameter to the GraphQL parser types, instead of making them 'static references
  • There should be a very simple command line or cargo command to run the sudograph tests against your own schema...that would be so cool. You just hit test and it will just start running the tests. In this way each project can have their own assurances about their own schemas, in addition to the random or specific schemas that the Sudograph project will be creating itself. These need to run fast so that people will want to run the tests on their own
  • This error seems to creep up very rarely in the order tests: thread 'test_order' panicked at 'called Option::unwrap() on a None value', src/arbitraries/order/order_read.rs:234:70
  • For ultimate robustness, we probably want to have many more individual tests...each capability should have an individual test, and then we should have combined tests on top
    • For example, for create we should test creating records with each individual type at a time, just test String in isolation, Boolean in isolation, etc. On top of that we would have the generalized test that we already have...it would be a lot of work, but I think it might be worth it to have the granularity and assurance. But key to all of this is being able to run the tests extremely quickly. They are very slow right now
  • Read search tests improvements
    • Having specific and generalized tests here might be really important
      • Test one field at a time and each search operation at a time
      • Test one field at a time and multiple search operations at a time
      • Test multiple fields at a time and multiple search operations at a time
      • This one could be broken down even more, I guess a line has to be drawn somewhere
  • limit, offset, order, search
  • I think it is very important to have the combination test as well, problems that might not manifest individually could manifest when combined. I might be running into one of these issues with the basic read tests
  • It would be amazing to combine all of the tests above into one test as well, which will generate queries with random combinations of limit, offset, order, search at arbitrary levels of the selection set. This could be pretty difficult (think about the complexity of the create and update tests vs the update disconnect and delete tests). The individual tests shouldn't actually be too hard to create, but combining them all could prove difficult. I am not sure if I should just start with the crazy combination test or start with the individual tests. Perhaps I should start with the individual tests and once I have two built, consider how to combine them and work from there. It will be good to have the individual tests so that we can easily pinpoint issues
  • Don't forget to test bytes in the selection set
  • This is possibly its own test...though I already have basic tests for this functionality in the update tests, and Blob is kind of an extra scalar that I wasn't originally going to include, so perhaps I will skip these tests for now
  • Actually I don't think I have any tests for this functionality, I have append and replace tests only, but I don't think I have limit or offset for bytes in the selection set, seems this should just be its own test
  • change and/or to use an array in the read search tests, and allow duplicate field values. Right now we are only testing the object syntax, which is weird
  • Test search or with array, we are only testing objects
  • enums should be added to limit, offset, and order I believe: I have not been testing enums with any of these tests. I don't know how long it has been since I have thought about enums, but we need to add them in
  • Panic if any graphql query or mutation returns an error in the order, limit, offset, search tests...well maybe just the mutation create mutation. The problem is if that fails, then the read objects will be empty and the tests will just pass
  • Manually test ors and ands within each other, especially empty ors within empty ors
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

1 participant