Skip to content

Conversation

@willnet
Copy link

@willnet willnet commented Oct 3, 2025

Add the latest Rails and Ruby to the CI matrix. CI then failed, so I made the following changes to get the tests passing:

  • Add version branching for sqlite3 that depends on Active Record 8.0
  • In Ruby 3.4, the inspect output changed, which broke tests that compare inspect results. I normalized the inspect output so those tests pass.

CI result is here.

@willnet
Copy link
Author

willnet commented Oct 24, 2025

Hi @apotonick—could you review this PR when you can? Thanks!

# Ruby 3.4 changed from {:key => value} to {key: value} in inspect output
def normalize_inspect(inspect_string)
inspect_string
.gsub(/" => /, '"=>') # Remove spaces around => for string keys: "key" => value → "key"=>value
Copy link
Owner

Choose a reason for hiding this comment

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

Cool, we have this https://github.com/trailblazer/trailblazer-core-utils/blob/master/lib/trailblazer/core/utils/inspect.rb#L14 which we could use here, but I will test your version! 💚

Would you mind making the tests assert_equal asserted, expected?

Copy link
Author

Choose a reason for hiding this comment

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

For now, I changed only the parts related to this diff to use the assert_equal expected, actual format. Let me know if you want me to update all the other tests as well.

Copy link
Author

Choose a reason for hiding this comment

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

@apotonick Any thoughts on this?

Copy link
Owner

Choose a reason for hiding this comment

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

Hi @willnet, thanks! I tested your gsub magic! 🎉 I will release trailblazer-core-utils soon and we can start using our inspect helper here. Are you happy to change the tests accordingly?

Copy link
Author

Choose a reason for hiding this comment

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

@apotonick Done! 😃

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