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

Refining and restructuring the test cases #266

Merged
merged 1 commit into from
Apr 1, 2021

Conversation

Turnerj
Copy link
Collaborator

@Turnerj Turnerj commented Mar 29, 2021

Changes

  • Type-specific tests (Book, MusicEvent, Website) are now in an "Examples" folder and namespace (the namespace change is useful when using the test explorer as they have their own expand/collapse node)
  • Type-specific tests no longer have a dedicated ToString test (avoids issues with JSON property order) however a round trip conversion test (convert to JSON and back to object) still exists for each proving that the ToString method works.
  • MixedTypesTest has moved to examples folder (mainly because the logic behind it is tested in ValuesJsonConverterTest however seemed worth keeping as an example)
  • Core parts of SerializationTest have been moved into ThingTest with SerializationTest being deleted
  • All tests use SchemaSerializer instead of directly using JsonConvert (this is the ideal way people will serialize our types and is what we directly call in Thing.ToString())

Overall nothing overly major has changed one way or another but it should eliminate the issue in #100 where tests failed due to JSON order while making a few other bits a little clearer.

@Turnerj Turnerj requested a review from RehanSaeed March 29, 2021 04:12
@RehanSaeed
Copy link
Owner

RehanSaeed commented Mar 29, 2021

Seems reasonable. If we're going to mess with tests, perhaps we can also choose a snapshot testing tool. There are a lot of options out there since snapshot testing in .NET seems to be in its infancy. I have started to evaluate some in https://twitter.com/RehanSaeedUK/status/1375470767475982336?s=20. So far, I'm leaning towards Verify but still doing some reading.

@Turnerj
Copy link
Collaborator Author

Turnerj commented Mar 29, 2021

Hmmm, I was looking into Verify but I'm not sure exactly how it (or tools like it) would fit in with what we have. Like you mentioned in #100, what we have for a number of the tests are good examples for different things - that's fine, just not sure where tests using a snapshot testing tool help us.

Happy to give it a shot though might need to understand more where it fits in and how.

@RehanSaeed
Copy link
Owner

Hmmm, I thought a snapshot tool would be useful to get rid of the JSON strings but now that I look at it, because we do serialize and deserialize tests, we need the JSON string. Ok, ignore me then.

@Turnerj
Copy link
Collaborator Author

Turnerj commented Apr 1, 2021

Any other thoughts with this PR? Happy to have it merged?

@RehanSaeed
Copy link
Owner

LGTM

@Turnerj Turnerj merged commit c41e0ad into RehanSaeed:main Apr 1, 2021
@RehanSaeed
Copy link
Owner

Good time to release?

@Turnerj
Copy link
Collaborator Author

Turnerj commented Apr 1, 2021

Sounds good to me! Will do some more stuff with S.T.J soon and maybe open a PR to support pending types like we discussed.

@Turnerj Turnerj deleted the refined-tests branch April 1, 2021 12:47
@RehanSaeed RehanSaeed added enhancement Issues describing an enhancement or pull requests adding an enhancement. patch Pull requests requiring a patch version update according to semantic versioning. labels Apr 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues describing an enhancement or pull requests adding an enhancement. patch Pull requests requiring a patch version update according to semantic versioning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants