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

First working version #149

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

First working version #149

wants to merge 18 commits into from

Conversation

idg10
Copy link
Contributor

@idg10 idg10 commented Jan 9, 2023

This library has hitherto been in a bit of an experimental state. It was using an old NuGet package for System.Text.Json instead of the implementation built into the runtime. It didn't handle nullable reference types well. And it was quite inconsistent with the Newtonsoft implementation of the same abstractions.

This PR addresses these issues, with the goal of making this library usable in practice for the first time.

* update to .NET 6.0
* no longer using the old System.Text.Json NuGet package - now using the one in the .NET runtime libraries
* property bags now created and modified through factory just like with the old Newtonsoft implementation
* property bag implementation no longer public (propagation of serializer options means you get into all sorts of problems if people can create these without using the factory)
* made specs more consistent with those in the Newtonsoft-based implementation to smooth the transition
Add queue-time build variable for forcing preview release
…gured in Azure DevOps instead - the yaml was just overriding it!)
I had mostly changed these to just Json, but I missed AddJsonDateTimeOffsetToIso8601AndUnixTimeConverter
AddJsonSerializerSettingsProvider -> AddJsonSerializerOptionsProvider

The old name was the same as the one used by the Newtonsoft library. This is unhelpful because in some test scenarios we need to be able to call both (until such time as everything has moved over to System.Text.Json). And in any case, the thing it provides is called JsonSerializerOptions.
Also define new JsonElement-friendly methods to IJsonPropertyBagFactory. (Not yet implemented.)
Also refactored tests to fix these problems:
 * most of the steps were in one massive class
 * lots of things were going through the black hold that is the ScenarioContext
 * the use of the per-ScenarioContext DI container wasn't working because tests needed to modify the DI setup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant