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

assert_equal_hash makes large hash comparisons easier to understand #5306

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

martinemde
Copy link
Member

@martinemde martinemde commented Dec 3, 2024

Two differently sorted hashes are equal in the eyes of assert_equal, but their diff output can be a huge mess due to the order of hashes (which does not affect equality).

This PR adds a method that sorts any hashes in expected and actual before comparing them in order to improve the readability of the output when they are not equal.

Also converts any multi-line hash comparisons in tests to use the new method.

@martinemde martinemde requested a review from segiddins December 3, 2024 19:29
@martinemde
Copy link
Member Author

Example failure before:

  1) Failure:
Avo::UsersSystemTest#test_change_user_email [test/system/avo/users_test.rb:522]:
Expected equal elements in expected.
--- expected
+++ actual
@@ -1,16 +1,18 @@
 {"records"=>
   {"gid://gemcutter/User/78070"=>
     {"changes"=>
-      {"updated_at"=>["2024-12-03T20:08:36.805Z", "2024-12-03T20:08:37.715Z"],
-       "email"=>["[email protected]", "[email protected]"],
+      {"email"=>["[email protected]", "[email protected]"],
+       "token_expires_at"=>
+        ["2024-12-03T23:08:36.805Z", "2024-12-03T23:08:37.711Z"],
        "email_confirmed"=>[true, false],
        "confirmation_token"=>
         ["5295949011b084b54aa2ee4dbace5e858973bc9f1e1946d9",
          "80cbfcc7c7ffb4b4186aa6249c638c5390ca1e8385ab2903"],
-       "token_expires_at"=>
-        ["2024-12-03T23:08:36.805Z", "2024-12-03T23:08:37.711Z"]},
+       "updated_at"=>["2024-12-03T20:08:36.805Z", "2024-12-03T20:08:37.715Z"]},
      "unchanged"=>
-      {"id"=>78070,
+      {"unconfirmed_email"=>nil,
+       "mfa_level"=>"disabled",
+       "id"=>78070,
        "encrypted_password"=>
         "$2a$04$OJFcViJPWq1Ru1yFi5KigeBs./Fg0HdOMF8EbZxC9GD5mbWFzp7qG",
        "salt"=>nil,
@@ -22,9 +24,7 @@
        "handle"=>"handle1",
        "hide_email"=>true,
        "twitter_username"=>nil,
-       "unconfirmed_email"=>nil,
        "remember_token_expires_at"=>nil,
-       "mfa_level"=>"disabled",
        "mail_fails"=>0,
        "blocked_email"=>nil,
        "webauthn_id"=>
@@ -46,41 +46,14 @@
         [nil,
          {"email"=>nil,
           "user_agent_info"=>
-           {"os"=>"Mac OS X",
+           {"installer"=>"Browser",
             "device"=>"Mac",
-            "system"=>nil,
-            "installer"=>"Browser",
+            "os"=>"Mac OS X",
             "user_agent"=>"HeadlessChrome",
-            "implementation"=>nil}}],
+            "implementation"=>nil,
+            "system"=>nil}}],
        "created_at"=>[nil, "2024-12-03T20:08:37.729Z"],
        "updated_at"=>[nil, "2024-12-03T20:08:37.729Z"]},
-     "unchanged"=>{}},
-   "gid://gemcutter/Events::UserEvent/97495"=>
-    {"changes"=>
-      {"id"=>[nil, 97495],
-       "tag"=>[nil, "user:email:sent"],
-       "trace_id"=>[nil, "0"],
-       "user_id"=>[nil, 78070],
-       "ip_address_id"=>[nil, 6797],
-       "geoip_info_id"=>[nil, 811],
-       "additional"=>
-        [nil,
-         {"to"=>"[email protected]",
-          "from"=>"[email protected]",
-          "action"=>"email_confirmation",
-          "mailer"=>"mailer",
-          "subject"=>"Please confirm your email address with RubyGems.org",
-          "message_id"=>
-           "[email protected]",
-          "user_agent_info"=>
-           {"os"=>"Mac OS X",
-            "device"=>"Mac",
-            "system"=>nil,
-            "installer"=>"Browser",
-            "user_agent"=>"HeadlessChrome",
-            "implementation"=>nil}}],
-       "created_at"=>[nil, "2024-12-03T20:08:37.948Z"],
-       "updated_at"=>[nil, "2024-12-03T20:08:37.948Z"]},
      "unchanged"=>{}}},
  "fields"=>{"from_email"=>"[email protected]"},
  "arguments"=>{},

With this change:

  1) Failure:
Avo::UsersSystemTest#test_change_user_email [test/system/avo/users_test.rb:522]:
Expected equal elements in expected.
--- expected
+++ actual
@@ -23,33 +23,6 @@
        "updated_at"=>[nil, "2024-12-03T20:18:59.654Z"],
        "user_id"=>[nil, 78103]},
      "unchanged"=>{}},
-   "gid://gemcutter/Events::UserEvent/97561"=>
-    {"changes"=>
-      {"additional"=>
-        [nil,
-         {"action"=>"email_confirmation",
-          "from"=>"[email protected]",
-          "mailer"=>"mailer",
-          "message_id"=>
-           "[email protected]",
-          "subject"=>"Please confirm your email address with RubyGems.org",
-          "to"=>"[email protected]",
-          "user_agent_info"=>
-           {"device"=>"Mac",
-            "implementation"=>nil,
-            "installer"=>"Browser",
-            "os"=>"Mac OS X",
-            "system"=>nil,
-            "user_agent"=>"HeadlessChrome"}}],
-       "created_at"=>[nil, "2024-12-03T20:18:59.862Z"],
-       "geoip_info_id"=>[nil, 832],
-       "id"=>[nil, 97561],
-       "ip_address_id"=>[nil, 6818],
-       "tag"=>[nil, "user:email:sent"],
-       "trace_id"=>[nil, "0"],
-       "updated_at"=>[nil, "2024-12-03T20:18:59.862Z"],
-       "user_id"=>[nil, 78103]},
-     "unchanged"=>{}},
    "gid://gemcutter/User/78103"=>
     {"changes"=>
       {"confirmation_token"=>

@martinemde martinemde changed the title assert_same_hash makes large hash comparisons easier to understand assert_equal_hash makes large hash comparisons easier to understand Dec 3, 2024
@martinemde martinemde force-pushed the martinemde/assert-same-hash branch from 63e504d to b04b418 Compare December 3, 2024 20:28
@martinemde
Copy link
Member Author

For reference, this seems not desired upstream.

@martinemde martinemde force-pushed the martinemde/assert-same-hash branch 3 times, most recently from 876a1cc to 3c23a18 Compare December 3, 2024 22:04
@martinemde
Copy link
Member Author

ok, the changes needed a bunch of indentation fixes, but nothing changed except getting the hashes into the assertion in a clean way.

Convert all multi-line hash comparisons to assert_equal_hash
@martinemde martinemde force-pushed the martinemde/assert-same-hash branch from 3c23a18 to f4e52e8 Compare December 3, 2024 22:06
Copy link

codecov bot commented Dec 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.14%. Comparing base (9a209b2) to head (f4e52e8).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5306   +/-   ##
=======================================
  Coverage   97.14%   97.14%           
=======================================
  Files         457      457           
  Lines        9564     9564           
=======================================
  Hits         9291     9291           
  Misses        273      273           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@indirect indirect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, thanks for improving this

@indirect indirect merged commit a786900 into master Dec 4, 2024
19 checks passed
@indirect indirect deleted the martinemde/assert-same-hash branch December 4, 2024 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants