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

Don't fail on properties with null values if they doesn't exist in the .snap file #170

Open
gdoron opened this issue Nov 28, 2022 · 8 comments
Milestone

Comments

@gdoron
Copy link

gdoron commented Nov 28, 2022

Often times developers use:

Snapshot.Match(dto);
which is nice, but when a new property is added to the dto, previous UTs, that weren't really affected by the change fail, because newPropery: null doesn't exist in the snap file.

Is there a way to achieve that?
We are seriously considering removing the library completely because we couldn't find a way to doing that.

@nscheibe
Copy link
Member

Hi gdoron,
We compare usually the original snapshot with the snapshot of the new object. And if there is a new property (also with value null), then it is serialized to json and the match will fail.

At the moment we have only the following:

  • Exclude Match Option, which excludes the new field. (Not really practical in your case)
  • You could add your own serialization settings of Newtonsoft to serialize it (and ignore new fields).

I think what you would need is a Global Setting, where you could enable a setting like "AcceptAllNewFields: true", and if this is set, then always if a new field is added to the snapshot it is automatically accepted. And maybe it should overwrite automatically the original snapshot?
Or maybe "AcceptAllNewNullFields", which just accepts a new field if it is null....

Let me know what you think. Maybe you have also some good ideas.

@gdoron
Copy link
Author

gdoron commented Nov 29, 2022

The second option is probably better, adding the new property to the file automatically means very little if it runs in the ci.

In the meantime how can we use a custom serializer? Our default newtonsoft serializer setting is filtering out null properties, so this might be a great temp workaround!

Thanks for the speedy response!
Doron

@nscheibe nscheibe added this to the 1.0.0 milestone Nov 29, 2022
@nscheibe
Copy link
Member

nscheibe commented Nov 29, 2022

Okay, we will add the AcceptNewNullFields to our feature list.

However our default json serialization settings are in the SnapshotSerializerSettings:

public static JsonSerializerSettings DefaultJsonSerializerSettings =>
            new JsonSerializerSettings
            {
                ReferenceLoopHandling = ReferenceLoopHandling.Ignore,
                Formatting = Formatting.Indented,
                NullValueHandling = NullValueHandling.Include,                      <-- 
                DateFormatHandling = DateFormatHandling.IsoDateFormat,
                Culture = CultureInfo.InvariantCulture,
                ContractResolver = ChildFirstContractResolver.Instance,
                Converters = new List<JsonConverter> {new StringEnumConverter()}                
            }; 

Now we have the possibility to overwrite this settings. Just create a class which inherits from SnapshotSerializerSettings and there you can extend our default settings (extend means in your case overwrite).

public class IgnoreNullValueSetting : SnapshotSerializerSettings
{
    public override JsonSerializerSettings Extend(JsonSerializerSettings settings)
    {
        settings.NullValueHandling = NullValueHandling.Ignore;
        return settings;
    }
}

Snapshooter searches within the AppDomain for all classes which are inherited from SnapshotSerializerSettings, then the default settings will be overwritten or extended during runtime.

I hope this can be used as a workaround in your case. Let me know if it worked.

@gdoron
Copy link
Author

gdoron commented Nov 30, 2022

I tried using SnapshotSerializerSettings, seems like that's won't exactly do it for us.
The problem is, people also add non nullable properties like integers, guids and booleans.

I think we should have a way of instructing snapshooter to simply not fail on new properties.
What do you think? @nscheibe

@glucaci
Copy link
Member

glucaci commented Dec 1, 2022

Hi @gdoron,

This is kind of strange use-case.
Do you never update the snapshoot with the new properties?

If you don't and you are interested only in couple of fields from your object than you can use the IncludeField. Take a look on this tests.

@gdoron
Copy link
Author

gdoron commented Dec 2, 2022

@glucaci we usually don't update.
We surely can use IncludeFields, and write down all the relevant fields, but if we can that path, we can just use
n times Assert.Equal since we anyway list the properties, it doesn't save much.

But I can totally understand where you're coming from.
I'm curious how much control jest provides over this.

@nscheibe
Copy link
Member

nscheibe commented Dec 2, 2022

I think we could add some parameters to the match option "IgnoreFields" that if the value is null or fullfills a condition, then it will be ignored, instead of using a path. And I gona check if we can set the MatchOptions somewhere globally, that they will be used for each snapshot match. I will have a look this weekend if this is possible to implement.

@glucaci
Copy link
Member

glucaci commented Dec 3, 2022

@gdoron is true if you have a flat object which is normally not the case in most of the applications. Most of the cases the property of a root object is a complex one which can have n-properties down on the tree, so it's not really comparable with Assert.Equal.

But yes, maybe some IgnoreNullOrDefault generic configuration will be helpful in some cases where you want to ignore all new null properties or default values of the value types.

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

No branches or pull requests

3 participants