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

PR #26 introduced exclude bug #27

Closed
digitaldirk opened this issue Jun 12, 2024 · 2 comments
Closed

PR #26 introduced exclude bug #27

digitaldirk opened this issue Jun 12, 2024 · 2 comments

Comments

@digitaldirk
Copy link

@replaysMike @MohammadHadi2031 Thank you for the fast PR and Merge, but I believe that #26 introduced a bug :(. Nested exclude strings not do not exclude the properties now.

I have complex objects that have nested collection properties, I use a list of strings to exclude certain properties (".positions.name", ".positions.locations.id",...) but now they are not excluded, only ones like ".Id", ".Name" correctly exclude.

Here is a simple test that passes on fe9c29f (Previous latest version) but fails on d03f685 (Latest version)
.Id is excluded correctly, but .Bs.Name is not

[Test]
public void ExcludeByStringTest()
{
    var B1s = new List<B> { new B { Id = 10, Name = "Test 10" }, new B { Id = 11, Name = "Test 11" } };
    var B2s = new List<B> { new B { Id = 10, Name = "Test 10" }, new B { Id = 12, Name = "Test 12" } };
    var A1 = new A();
    A1.Id = 1;
    A1.Bs = B1s;
    var A2 = new A();
    A1.Id = 2;
    A2.Bs = B2s;

    var diff = A1.Diff(A2, propertiesToExcludeOrInclude: new string[] { ".Bs.Name", ".Id"});

    Assert.AreEqual(1, diff.Count);
}

class A
{
    public List<B> Bs { get; set; } = new List<B>();
    public int Id { get; set; }
}

class B
{
    public int Id { get; set; }
    public string Name { get; set; }
}
@MohammadHadi2031
Copy link
Contributor

Thanks for your feedback. I will check it soon

MohammadHadi2031 added a commit to MohammadHadi2031/AnyDiff that referenced this issue Jun 16, 2024
MohammadHadi2031 added a commit to MohammadHadi2031/AnyDiff that referenced this issue Jun 16, 2024
@replaysMike
Copy link
Owner

Indeed it did, I was in a rush that day and didn't double check the tests had passed before pushing a release. Looks like the current build scripts don't detect when the tests fail 😅

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