search

thoughtbot / shoulda-matchers

Simple one-liner tests for common Rails functionality
https://matchers.shoulda.io
MIT License
3.51k stars 912 forks source link

Rails 7.2 compatibility #1632

Closed theodorton closed 2 months ago

theodorton commented 4 months ago

Supersedes #1631

I've been running tests against various git versions of Rails for one of my projects and noticed there are a couple things breaking in shoulda-matchers. This led me to try running the shoulda-matcher tests against Rails 7.2 and I discovered some things that were no longer compatible. This PR should resolve the previously failing tests.

Let me know if you'd like me to refactor and/or reword anything here.

I understand if you want to hold off on merging this until 7.2 is officially released. I can update the Appraisal and rebase when it's out.

theodorton commented 4 months ago

Rubocop issues should be resolved.

matsales28 commented 4 months ago

@theodorton Thanks for working on that! The overall changes look good to me. Feel free to add the new appraisal to the workflow file so we can run the CI with the new rails version. https://github.com/thoughtbot/shoulda-matchers/blob/main/.github/workflows/ci.yml

Also, could you add specs for the counter_cache change? The syntax has changed a bit. I think it's better to wait for the official release as if they decide to change more stuff, there might be other necessary changes.

Thanks again for working on that!

theodorton commented 4 months ago

@matsales28 I added specs for counter_cache in 4224dec, and acceptance specs should pass with the latest commits. I'll rebase to clean up once you feel it's time to merge.

spickermann commented 2 months ago

Ruby on Rails 7.2.0 was officially released a few hours ago. Is there anything I can do to help to get this PR merged and deployed?

matsales28 commented 2 months ago

Hi @theodorton, Thank you for your work on the PR! With the new Rails version now officially released, it would be great if you could continue with the adjustments.

@spickermann has also offered to help move this PR forward. If you're open to collaborating. I'm happy to assist as well—just let me know if there's anything I can do to support you. If you don't have time now to do that, please let me know and I can move this PR forward myself.

I really appreciate the effort!

theodorton commented 2 months ago

@matsales28 I'll spend some time today to bring it up to date with the official release and see if I can resolve the failing specs. I will let you know if I need help moving it forward.

theodorton commented 2 months ago

@matsales28 I've cleaned up the history here and believe I've resolved all the issues that surfaced from the previous build.

matsales28 commented 2 months ago

@matsales28 I've cleaned up the history here and believe I've resolved all the issues that surfaced from the previous build.

CI passed. I'll take another look at the code later, but it's looking good. If nothing extraordinary happens, I'll probably merge it today, and release a new version. Thanks again for working on this!