Skip to content

feat: date like objects support#198

Merged
mcmire merged 8 commits intosplitwise:mainfrom
fizvlad:feat/date-like-objects-support
Jan 30, 2024
Merged

feat: date like objects support#198
mcmire merged 8 commits intosplitwise:mainfrom
fizvlad:feat/date-like-objects-support

Conversation

@fizvlad
Copy link
Contributor

@fizvlad fizvlad commented Oct 14, 2023

Closes #182

Tbh, I just went through appearances of /(TimeLike|time_like)/ and created similar logic for date like objects :D

I'm really hesitant about whether DateTime objects should be considered date_like (although ActiveSupport defines DateTime#acts_like_date?). Since Date and DateTime are never equal, expect(Date.new(2020, 1, 1)).to eq(DateTime.new(2020, 1, 1)) will result in a confusing diff (see the spec/support/shared_examples/active_support.rb)

@fizvlad
Copy link
Contributor Author

fizvlad commented Nov 8, 2023

@mcmire, could you take a look?

@mcmire
Copy link
Collaborator

mcmire commented Nov 13, 2023

Hey @fizvlad! Thanks for the PR. I've looked over it and nothing stands out, but haven't had time to play with it yet.

About the confusing diff — is there anything you feel like should happen differently there?

@fizvlad
Copy link
Contributor Author

fizvlad commented Nov 14, 2023

About the confusing diff — is there anything you feel like should happen differently there?

I'm only concerned about diff making it feel like day: 14 instead of day: 31 is the only problem, while classes of expected and actual also matter. Maybe highlighting that objects got different classes would make diff better. As a user I probably wouldn't even notice this behavior, so I'm totally fine with not doing any changes :D

Current output:

       Actual output:

       ---START----------------------------------------------------------------
       F

       Failures:

         1) test passes
            Failure/Error: expect(expected).to eq(actual)

              Expected #<Date 2023-10-14> to eq #<DateTime 2023-10-31 18:22:26 +00:00 (+00:00)>.

              Diff:

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

                #<Date {
                  year: 2023,
                  month: 10,
              -   day: 31
              +   day: 14
                }>
            # ./lib/super_diff/rspec/monkey_patches.rb:43:in `handle_failure'
            # ./tmp/integration_spec.rb:16:in `block (2 levels) in <top (required)>'

       Finished in 0.02233 seconds (files took 0.575 seconds to load)
       1 example, 1 failure

       Failed examples:

       rspec ./tmp/integration_spec.rb:13 # test passes
       ---END------------------------------------------------------------------

@fizvlad fizvlad force-pushed the feat/date-like-objects-support branch from 7f1d816 to 4dc1800 Compare November 14, 2023 07:45
@fizvlad
Copy link
Contributor Author

fizvlad commented Nov 14, 2023

Rebased the branch to resolve merge conflicts

@fizvlad fizvlad force-pushed the feat/date-like-objects-support branch from 4dc1800 to a0decaf Compare November 18, 2023 16:08
@fizvlad
Copy link
Contributor Author

fizvlad commented Nov 18, 2023

Specs with no_rails were failing since date is not required by default (resolved by adding require "date") and DateTime only have special inspect with ActiveSupport (moved specs to active_support folfer)

@fizvlad
Copy link
Contributor Author

fizvlad commented Dec 13, 2023

Hey, @mcmire ! Could you take another look?

@mcmire mcmire merged commit 4965ac0 into splitwise:main Jan 30, 2024
@mcmire
Copy link
Collaborator

mcmire commented Jan 30, 2024

Hey @fizvlad! Sorry for the long delay on this. I think I can fix that diff issue you mentioned but the rest of this seemed fine! Thanks again for the PR.

@fizvlad fizvlad deleted the feat/date-like-objects-support branch January 30, 2024 09:25
@mcmire
Copy link
Collaborator

mcmire commented Feb 10, 2024

This feature has been included in v0.11.0. Thanks again!

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.

Failed Assertions on Date Instances Do Not Output a Diff

2 participants