Conversation
Pull Request Test Coverage Report for Build c5b80b89-03d0-407b-9513-028652dfebe4Details
💛 - Coveralls |
david-yz-liu
left a comment
There was a problem hiding this comment.
Hi @Purplegaze, you're definitely on the right track!
Regarding creating instances of Text, Path, and Shape, you can see the original definitions of these data types in app/Database/Table.hs. The syntax is a bit different from the normal type definitions because it's using Persistent's schema syntax, but the principle is the same: each type can be treated as a record and can be created just by passing in values of the correct types for each field. The only tricky one is GraphId - you should be able to use toSqlKey 1 as the value for this (don't worry about the exact key value, as it'll be overridden in insertGraph).
In terms of the assertion, you can try decoding the response string into an SvgJSON instance, like what saveGraphJSON does.
Proposed Changes
Draft PR to add
getGraphJSONto theControllers.Graphtest suite.There are currently no test cases other than the empty case. I'll make this PR ready for review once those are added (see Questions section)
...
Screenshots of your changes (if applicable)
Type of Change
(Write an
Xor a brief description next to the type or types that best describe your changes.)Checklist
(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the
[ ]into a[x]in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)Before opening your pull request:
After opening your pull request:
Questions and Comments
This is a draft PR until I populate
getGraphJSONTestCasesbeyond the empty case.I'm not sure whether I should do this via building the shapes directly (which I'll need to figure out, with
Svg.BuilderI assume), or directly via JSON import like in thesaveGraphJSONtests. I think the former would be better since getGraphJSON and saveGraphJSON tests sound like they should be separate from each other, but I'd like confirmation on what direction to go here.The assertEqual statement is just a string check at the moment, but it would be cleaner to change it to parse the relevant JSON fields directly (with one assert statement for each relevant part of the response body, if I do the former option of building the shapes directly)