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

Support for 'change X from A to B' #127

Open
jesseduffield opened this issue Apr 16, 2021 · 3 comments
Open

Support for 'change X from A to B' #127

jesseduffield opened this issue Apr 16, 2021 · 3 comments

Comments

@jesseduffield
Copy link

First off, thanks for this project, it's been a great help to me :)

I was wondering if we could add support for when we assert both on the start and end value of an object. This is the norm at my workplace because it ensures that the end value isn't coincidentally the start value due to let bindings being shuffled around. E.g:

arr = [1, 2]
arr << 1
expect(arr).to eq [1, 2, 3]

yields:

       Expected [1, 2, 1] to eq [1, 2, 3].

       Diff:

       ┌ (Key) ──────────────────────────┐
       │ ‹-› in expected, not in actual  │
       │ ‹+› in actual, not in expected  │
       │ ‹ › in both expected and actual │
       └─────────────────────────────────┘

         [
           1,
           2,
       -   3
       +   1,
         ]

but

arr = [1, 2]
expect { arr << 1 }.to change { arr }.from([1, 2]).to([1, 2, 3])

yields:

expected `arr` to have changed to [1, 2, 3], but is now [1, 2, 1]

Ideally this would show a diff of the difference if either the start value or end value don't match up with the actual value when checked.

If this is something I can help contribute to, let me know :)

@mcmire
Copy link
Collaborator

mcmire commented Apr 16, 2021

Hey there! Glad this gem is helping you, and thanks for the suggestion. You mention that you want the diff to show up if the start or end of the value in question is different, but I'm curious whether it would be more helpful to people (and less surprising) if the logic were more general — display the diff of the "before" and "after" versions if they don't match at all? For instance, what if the two arrays are technically the same at the start or end but different in the middle:

arr = [1, 2, 3, 4]
expect { }.to change { arr }.from([1, 2, 3, 4]).to([1, 'a', 'b', 4])

Or what if one or both of the values aren't arrays at all:

arr = [1, 2, 3, 4]
expect { arr = :foo }.to change { arr }.from([1, 2, 3, 4]).to([1, 'a', 'b', 4])

It seems like it might still be helpful to show the diff in those cases. What do you think?

@jesseduffield
Copy link
Author

Yep that works for me.

Also, its good to see a fellow commander keen fan in the wild

@mcmire
Copy link
Collaborator

mcmire commented Apr 16, 2021

@jesseduffield Haha, thanks! Many people don't recognize it so it's cool you noticed.

If you want to dive into working on a fix for this, then that would be neat! If you want to understand how the gem is structured, I have a visual and some quick explanations of concepts here. That said, I know the documentation is lacking, so here are some pointers:

  1. You will at the very least want to override RSpec::Matchers::BuiltIn::Change in this file, and add the AugmentedMatcher module into that class by saying SuperDiff.insert_overrides(self, SuperDiff::RSpec::AugmentedMatcher). This module will give the change matcher the ability to show a custom diff and will get you 75% of the way there.
  2. The remaining 25% will probably be around figuring out how to present failure messages, given that change takes a block and eq does not. There are some methods you can override (again provided by AugmentedMatcher) that let you do this. You may have to make a custom MatcherTextBuilder class. You can see an example of this for the be_* matcher here and how it is used here. (But maybe skip to the next step first.)
  3. In terms of tests, you could probably start by copying one of the first few tests for the eq matcher here to a new file dedicated to the change matcher. That will give you a playground to work out what the output for the matcher might look like in various situations.

I imagine you will probably need some help as I know not everything is super obvious, so don't hesitate to ask (either here or in a new draft PR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants