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

Tests against different activerecord/activesupport versions #109

Merged
merged 11 commits into from
Aug 20, 2024

Conversation

Aqualon
Copy link
Contributor

@Aqualon Aqualon commented Aug 15, 2024

Currently tests don't run against different activerecord versions, but just assumed everything is like in activerecord 5.

  • Use released appraisal gem, issue for using from github.com was released in 2.5.0
  • Test against 43 different activerecord versions [6.1.7.8, 7.0.8.4, 7.1.3.4] (7.2.0 fails currently, so would only enable it together with the fix)
  • Use appraisal generate to avoid installing all gem versions on all builds
  • Can we move out the go tests, since they are independent of the ruby/redis/activerecord matrix? Currently they run on all of them we can't, because failover tests require beetle binary created in the go step

The issue to use this commit was fixed with thoughtbot/appraisal@ea2baff
To lessen the load, I also switched from appraisal install to appraisal generate and then only install gems for a given generated gemfile, not for all of them.
…or faster results"

go is needed to create beetle binary, that is then used for cucumber tests

This reverts commit d127a7b.
… provided in docker compose up step"

Yes we do for beetle binary tests

This reverts commit 4c10ddc.
@Aqualon
Copy link
Contributor Author

Aqualon commented Aug 19, 2024

@danielgoncharov @david-krentzlin what do you think of this? Not sure about the cost side, with additional checks against activerecord versions we would end up with 3 * 2 * 4 = 24 checks eventually.

@jojahner
Copy link
Contributor

jojahner commented Aug 19, 2024

@Aqualon Yeah, I think it is a good idea and since they should run in parallel I see no problem at all. Wondering if we want this for activesupport as well.

@jojahner
Copy link
Contributor

It would also not increase the number of combinations, since they should come with the same version. @Aqualon What do you think?

@Aqualon
Copy link
Contributor Author

Aqualon commented Aug 19, 2024

activerecord always comes with activesupport as dependency in the same version. But maybe we should make it more explicit in the Appraisals/workflow, define rails_versions and then add activerecord and activesupport in the appraise block?

@jojahner
Copy link
Contributor

Ah, yeah true. I would make it explicit, and do it as you said. Define the ruby version and then add activerecord and activesupport.

@Aqualon Aqualon changed the title Tests against different activerecord versions Tests against different activerecord/activesupport versions Aug 19, 2024
@Aqualon
Copy link
Contributor Author

Aqualon commented Aug 19, 2024

@jojahner if it's fine for you, please approve/merge it. I'd then open another PR for the 7.2.0 adaptations

@jojahner
Copy link
Contributor

Cool, thank you! ❤️

@jojahner jojahner merged commit 5f55e85 into master Aug 20, 2024
18 checks passed
@jojahner jojahner deleted the adapt_to_rails_7_2 branch August 20, 2024 06:35
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