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

Export diff module stuff #24

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bkontur
Copy link

@bkontur bkontur commented Jun 4, 2021

Hello,

at first, I would like to very thank you for this library, we used it for our tests and it works perfect.

And second, we have one use case: "we need to compare two jsons to be exact, but we need to ignore some properties".
So, I needed to make diff module public, because I ended up with this implementation, which works for us.
I am fine to depends on our fork, but I would better depends on your library.
What do you think?

It is inspired by your assert_json_matches_no_panic:

        if !ignore_json_properties.is_empty() {
            assert_json_eq_no_panic_with_ignore_json_properties(
                lhs,
                rhs,
                assert_json_diff::Config::new(assert_json_diff::CompareMode::Strict),
                ignore_json_properties,
            )
        } else {
            assert_json_diff::assert_json_matches_no_panic(
                lhs,
                rhs,
                assert_json_diff::Config::new(assert_json_diff::CompareMode::Strict),
            )
        }
fn assert_json_eq_no_panic_with_ignore_json_properties<Lhs, Rhs>(
        lhs: &Lhs,
        rhs: &Rhs,
        config: assert_json_diff::Config,
        ignore_json_properties: &[String],
    ) -> Result<(), String>
    where
        Lhs: Serialize,
        Rhs: Serialize,
    {
        let lhs = serde_json::to_value(lhs).unwrap_or_else(|err| {
            panic!(
                "Couldn't convert left hand side value to JSON. Serde error: {}",
                err
            )
        });
        let rhs = serde_json::to_value(rhs).unwrap_or_else(|err| {
            panic!(
                "Couldn't convert right hand side value to JSON. Serde error: {}",
                err
            )
        });

        let diffs = assert_json_diff::diff::diff(&lhs, &rhs, config);

        if diffs.is_empty() {
            Ok(())
        } else {
            let diffs = diffs
                .into_iter()
                .filter(|diff| {
                    if let assert_json_diff::diff::Path::Keys(keys) = &diff.path {
                        if let Some(last_key) = keys.last() {
                            let ignore = ignore_json_properties.iter().any(|ijp| {
                                if let assert_json_diff::diff::Key::Field(field_name) = last_key {
                                    field_name.eq(ijp)
                                } else {
                                    false
                                }
                            });
                            if ignore {
                                println!();
                                println!(
                                    "Found IGNORED property, which does not matches, diff: {:?}",
                                    diff
                                );
                                false
                            } else {
                                true
                            }
                        } else {
                            true
                        }
                    } else {
                        true
                    }
                })
                .collect::<Vec<_>>();

            if diffs.is_empty() {
                Ok(())
            } else {
                let msg = diffs
                    .into_iter()
                    .map(|d| d.to_string())
                    .collect::<Vec<_>>()
                    .join("\n\n");
                Err(msg)
            }
        }
    }

@davidpdrsn
Copy link
Owner

Hm I'm not sure about this. I quite like that the API surface is currently as small as it is. Makes it easier to keep things stable. Making this public also feels like the "easy" solution. I wonder if your use case would be better addressed some other way. It seems quite special to me and is not something I've needed myself in the past.

@Mcdostone
Copy link

I agree with @bkontur, it's not very convienent for developers to deal with String messages.
Being able te play with the Difference struct would be better.

I don't have a specific use case (I just need to diff a lot of JSON objects and store the results in a database). I ended up with regexes to extract relevant information.

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.

3 participants