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

Added compact JSON format #288

Merged
merged 2 commits into from
Sep 26, 2022
Merged

Added compact JSON format #288

merged 2 commits into from
Sep 26, 2022

Conversation

mitsuhiko
Copy link
Owner

This adds a compact JSON format. For up to 120 characters the snapshot is put in a single line, beyond that multiple lines are emitted.

Refs #287

@mitsuhiko
Copy link
Owner Author

I had to bump up to 1.56 due to matklad/once_cell#201

@mitsuhiko
Copy link
Owner Author

@irevoire does this address your issue?

@irevoire
Copy link

Well, it's better as default behaviour, but don't you think being able to choose what's going to happen would be even better? 👀

@mitsuhiko
Copy link
Owner Author

What do you mean by "chose what's going to happen"?

@irevoire
Copy link

Having this PR as the default behaviour and then a compact and pretty macro I guess 🤔

@mitsuhiko
Copy link
Owner Author

You can always alias the macro yourself? Generally I think the compact behavior is likely inferior for most uses as it results in worse diffability.

@irevoire
Copy link

irevoire commented Sep 26, 2022

I can change the name, but I can't create a new macro that only uses the “default” view without forking insta, right?
After this PR, the best I'll be able to do is to use the compact view that stops working once I reach 120 chars?


Here is the kind of use-cases where it gets unreadable if you use the prettyfier;

    #[test]
    fn anything_and_index_deletion() {
        // The indexdeletion doesn't batch with anything that happens AFTER
        assert_smol_debug_snapshot!(autobatch(input_from([IndexDeletion, DocumentAddition])), @"Some(IndexDeletion { ids: [0] })");
        assert_smol_debug_snapshot!(autobatch(input_from([IndexDeletion, DocumentUpdate])), @"Some(IndexDeletion { ids: [0] })");
        assert_smol_debug_snapshot!(autobatch(input_from([IndexDeletion, DocumentDeletion])), @"Some(IndexDeletion { ids: [0] })");
        assert_smol_debug_snapshot!(autobatch(input_from([IndexDeletion, DocumentClear])), @"Some(IndexDeletion { ids: [0] })");
        assert_smol_debug_snapshot!(autobatch(input_from([IndexDeletion, Settings])), @"Some(IndexDeletion { ids: [0] })");

        // The index deletion can accept almost any type of BatchKind and transform it to an IndexDeletion
        // First, the basic cases
        assert_smol_debug_snapshot!(autobatch(input_from([DocumentAddition, IndexDeletion])), @"Some(IndexDeletion { ids: [0, 1] })");
        assert_smol_debug_snapshot!(autobatch(input_from([DocumentUpdate, IndexDeletion])), @"Some(IndexDeletion { ids: [0, 1] })");
        assert_smol_debug_snapshot!(autobatch(input_from([DocumentDeletion, IndexDeletion])), @"Some(IndexDeletion { ids: [0, 1] })");
        assert_smol_debug_snapshot!(autobatch(input_from([DocumentClear, IndexDeletion])), @"Some(IndexDeletion { ids: [0, 1] })");
        assert_smol_debug_snapshot!(autobatch(input_from([Settings, IndexDeletion])), @"Some(IndexDeletion { ids: [0, 1] })");

        // Then the mixed cases
        assert_smol_debug_snapshot!(autobatch(input_from([DocumentAddition, Settings, IndexDeletion])), @"Some(IndexDeletion { ids: [0, 2, 1] })");
        assert_smol_debug_snapshot!(autobatch(input_from([DocumentUpdate, Settings, IndexDeletion])), @"Some(IndexDeletion { ids: [0, 2, 1] })");
        assert_smol_debug_snapshot!(autobatch(input_from([DocumentAddition, Settings, DocumentClear, IndexDeletion])), @"Some(IndexDeletion { ids: [1, 3, 0, 2] })");
        assert_smol_debug_snapshot!(autobatch(input_from([DocumentUpdate, Settings, DocumentClear, IndexDeletion])), @"Some(IndexDeletion { ids: [1, 3, 0, 2] })");
    }

Honestly, the more I think about it, and the more I think you're right, 120 chars are probably enough for everyone; I just find it strange to force a limit instead of letting people assume their bad choices with an always compact view and an always prettified version 😁

@mitsuhiko
Copy link
Owner Author

I think the problem if it does not have some sort of auto detection is that you are forced to use different types of assertion macros all the sudden if the thing you assert on gets too large. I'm not sure that's a good user experience.

@irevoire
Copy link

That's true.
Well, thanks for the PR and sorry for thinking too loud. I actually think it totally fixes my « issue »!

@mitsuhiko mitsuhiko merged commit 7d74731 into master Sep 26, 2022
@mitsuhiko mitsuhiko deleted the feature/compact-json branch September 26, 2022 15:50
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.

2 participants