Skip to content

Conversation

@Purplegaze
Copy link

Proposed Changes

Draft PR to add getGraphJSON to the Controllers.Graph test 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 X or a brief description next to the type or types that best describe your changes.)

Type Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change that adds functionality)
🐛 Bug fix (non-breaking change that fixes an issue)
🎨 User interface change (change to user interface; provide screenshots)
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests) X
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

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:

  • I have performed a self-review of my changes.
    • Check that all changed files included in this pull request are intentional changes.
    • Check that all changes are relevant to the purpose of this pull request, as described above.
  • I have added tests for my changes, if applicable.
    • This is required for all bug fixes and new features.
  • I have updated the project documentation, if applicable.
    • This is required for new features.
  • If this is my first contribution, I have added myself to the list of contributors.
  • I have updated the project Changelog (this is required for all changes).

After opening your pull request:

  • I have verified that the CircleCI checks have passed.
  • I have requested a review from a project maintainer.

Questions and Comments

This is a draft PR until I populate getGraphJSONTestCases beyond 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.Builder I assume), or directly via JSON import like in the saveGraphJSON tests. 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)

@coveralls
Copy link

Pull Request Test Coverage Report for Build c5b80b89-03d0-407b-9513-028652dfebe4

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 55.445%

Totals Coverage Status
Change from base Build da33a828-ced2-4b81-8cb6-0099288bb4f2: 0.1%
Covered Lines: 2237
Relevant Lines: 3962

💛 - Coveralls

@Purplegaze Purplegaze changed the title Add getGraphJSON test template Add tests for getGraphJSON Jan 30, 2026
Copy link
Contributor

@david-yz-liu david-yz-liu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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

Successfully merging this pull request may close these issues.

3 participants