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

Add round-trip and save format tests #3

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

dsmiller95
Copy link

This PR adds tests which:

  • Assert basic round-trip serialize and deserialize functionality.
  • Assert json save format, ideally to make the specific json format more clear. and call out any changes clearly
  • Unique round-trip and json save format tests which run when Newtonsoft.Json-for-Unity.Converters package is present

The project used to develop these changes is here: https://github.com/dsmiller95/SaveAsyncEditingProject . Most tests will run in a fresh project, but to see the json-for-unity tests, the development project must also install the json-for-unity (and leave it at default settings).

These tests uncovered one potential issue, documented in #2

- there is no domain reload between tests
- the SaveManager stores state in a singleton, so it only resets on domain reload
- there is no way to deregister a saveable if it is destroyed
improve async -> coroutine by reading the UniTask library source code
…atible with running on different threads. instead relying on C# System.Threading.Tasks
organize test files. extract a shared function to a base test class

add some documentation to the tests
fixup the unity converter test based on real functionality
@dsmiller95 dsmiller95 marked this pull request as ready for review September 2, 2024 20:44
@nickpettit
Copy link
Member

@dsmiller95 I completely missed this PR! Sorry for the slow response, I'll take a look soon and write more. Thanks so much for contributing!

@nickpettit nickpettit self-assigned this Sep 23, 2024
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.

2 participants