Use `appraisal` gem in CI #1308

boardfish · 2022-03-14T17:27:01Z

Supersedes #1230

Uses appraisal ~> 2.5 to run against an array of Rails minor versions from 6.1 onwards. Also moves us away from manually caching gem dependencies, as that can now be handled by the setup-ruby action.

Spone · 2022-03-16T11:24:13Z

Looking good, thanks for wrapping this up! 🎉

While re-reading #1230, I noticed there was one last thing to decide. Do we want to:

merge this as it is, with the dependency on excid3/appraisal fork (see this for background)
wait for Use bundler original env when running bundle commands. Fixes #173 thoughtbot/appraisal#174 to be merged
refactor this PR to not use appraisal directly in CI, but setting BUNDLE_GEMFILE via matrix, as suggested here

Happy to hear what you think @joelhawksley @boardfish @BlakeWilliams

boardfish · 2022-03-16T21:19:33Z

I'm leaning towards that first option. I think we should prioritize ensuring that running the tests works the same way in dev as it does in CI. It may be a good idea in the long run to mirror @excid3's fork to @github so we can ensure that version of the code remains publicly available until @excid3's PR is merged.

camertron

Nice this is great! I'm a big fan of appraisal :)

camertron · 2022-03-24T17:59:51Z

.github/workflows/ci.yml

    runs-on: ubuntu-latest
    strategy:
      fail-fast: false
      matrix:
-        rails_version: ['5.2.6', '6.0.4.4', '6.1.4.4', '7.0.1', 'main']


Just a heads-up a repo admin will need to adjust the "required checks" after merging in order to accommodate the different labels here.

Spone · 2022-06-02T10:40:31Z

@joelhawksley would you mind giving us your feedback on this? (See my latest comment above). I think we should try to merge this soon.

joelhawksley

@Spone IMO we need to be cautious of depending on a long-running upstream PR on Appraisal.

Let's take another approach, or work to get the upstream PR merged.

allmarkedup · 2022-09-14T14:00:08Z

FWIW, I've just gone through the fun process of trying to get Lookbook CI working with Appraisal - I took a slightly different approach (and with a smaller matrix of ruby/rails versions for now) but I finally got it working. Just in case it's useful to have an example which uses the matrix approach but doesn't require the Appraisal fork: https://github.com/allmarkedup/lookbook/blob/main/.github/workflows/ci.yml

Spone · 2023-01-18T21:43:35Z

There is a new PR on Appraisal that could fix the issue and allow us to move forward with this approach.

If it does not, I'll take some time to mirror @allmarkedup's method.

nickcharlton · 2023-01-27T17:47:10Z

Appraisal maintainer here! 👋

I've just merged that PR mentioned above (thoughtbot/appraisal#202), which I'm hoping can help you out here. In a few days I'll cut a new release.

We'd been a bit stuck with a good solution to that problem, but let us know over there if you're having problems.

Spone · 2023-01-27T18:13:40Z

Awesome @nickcharlton! I'll try it out next week. Thanks for the heads up!

Spone · 2023-02-06T23:01:21Z

Gemfile

@@ -17,6 +17,10 @@ group :test do
  gem "selenium-webdriver", "~> 4"
 end

+group :development, :test do
+  gem "appraisal", github: "thoughtbot/appraisal"


I'm targeting main for now, but we'll be able to use the next version when it's released.

Spone · 2023-02-06T23:03:32Z

I thought it was time to get this in shape for v3.0.0 :)

Do not forget this after merging: #1308 (comment)

Thanks a lot @boardfish for your help on this!

Spone · 2023-02-23T19:11:59Z

Do you want to review this again @joelhawksley @camertron? Else I think I'll merge this in the coming days.

joelhawksley · 2023-02-23T19:29:26Z

@Spone I'd prefer we wait for the Appraisal release instead of depending on a branch.

joelhawksley · 2023-12-13T21:24:18Z

@boardfish thoughts on testing this against thoughtbot/appraisal#204 (comment)? Perhaps we don't need to wait for 3.0?

boardfish · 2023-12-14T16:28:01Z

Yeah, that sounds good!

joelhawksley · 2023-12-14T16:39:07Z

@boardfish cool, would you be up for bringing this PR up to date?

boardfish · 2023-12-14T16:40:05Z

Sure! I can have a crack at that sometime in the next week.

camertron

LGTM! Can you merge main?

camertron · 2024-01-02T18:38:26Z

Ugh sorry @boardfish, could you fix these merge conflicts? I'm not sure how to resolve the difference in the Gemfile 😓

gemfiles/rails_main.gemfile

joelhawksley · 2024-01-03T17:52:14Z

lib/view_component/test_helpers.rb

+        # :nocov:
        raise "`render_preview` expected a described_class, but it is nil." if described_class.nil?

        "#{described_class}Preview"
+        # :nocov:
      else
        self.class.name.gsub("Test", "Preview")
      end
      result = result.constantize
    rescue NameError
      raise NameError, "`render_preview` expected to find #{result}, but it does not exist."
    end
+    # :nocov:


Why were these changes necessary? I'd really prefer to avoid adding these comments.

I'll try dropping them, but runs were failing due to coverage errors. I think this was the cause, though I'm not sure if anything in this PR in particular would've triggered it.

.github/workflows/ci.yml

camertron · 2024-01-03T18:07:49Z

.github/workflows/ci.yml

-    - uses: actions/cache@v3
-      with:
-        path: vendor/bundle
-        key: gems-build-rails-${{ matrix.rails_version }}-ruby-${{ matrix.ruby_version }}-${{ hashFiles('**/Gemfile.lock') }}
+        bundler-cache: true


boardfish · 2024-01-04T09:21:35Z

@joelhawksley Looks like we do still get coverage-related failures without those nocov comments. Not sure why, though – maybe there's a discrepancy in the running environment between the runs on main vs. on this branch.

I feel like we should merge this and follow up to resolve that. What do you reckon?

joelhawksley · 2024-01-04T16:49:39Z

@boardfish I'm happy to merge and revisit given the complexity of this PR.

boardfish · 2024-01-06T10:02:28Z

Awesome. I'll get rid of those merge conflicts and merge this in myself later, then 👍

Appraisal allows us to test with different versions of gems. In addition to GitHub Actions' matrix functionality, we can use this to test against different combinations of Ruby and Rails versions. As part of this, we've also made the following changes to our CI pipeline: Rename Appraisal rails-head to rails-main Loosen version constraints to minor versions Use setup-ruby's caching mechanism Means we don't need to configure vendor/bundle as the path here. We should probably do this for other workflows too. Don't trigger CI twice on PRs Co-authored-by: Simon Fish <si@mon.fish>

boardfish changed the title ~~Fix appraisal ci boardfish~~ Fix appraisal in CI Mar 14, 2022

boardfish marked this pull request as ready for review March 15, 2022 10:42

boardfish requested review from Spone and a team as code owners March 15, 2022 10:42

Spone mentioned this pull request Mar 16, 2022

Use Appraisal in CI #1230

Closed

camertron approved these changes Mar 24, 2022

View reviewed changes

joelhawksley requested changes Jun 8, 2022

View reviewed changes

Spone requested review from joelhawksley and camertron February 6, 2023 22:58

Spone reviewed Feb 6, 2023

View reviewed changes

Spone self-assigned this Feb 6, 2023

Spone mentioned this pull request Feb 6, 2023

Add test coverage for use of turbo_stream helpers in components #1227

Merged

Spone mentioned this pull request Feb 18, 2023

all_view_paths undefined for ActionView::ViewPaths:Module lookbook-hq/lookbook#334

Closed

joelhawksley added the blocked label Apr 4, 2023

boardfish self-assigned this Dec 15, 2023

boardfish removed the blocked label Dec 15, 2023

boardfish force-pushed the fix-appraisal-ci-boardfish branch 2 times, most recently from d1d26ad to edb5fb7 Compare December 17, 2023 11:15

boardfish added the github_actions Pull requests that update GitHub Actions code label Dec 18, 2023

boardfish force-pushed the fix-appraisal-ci-boardfish branch from edb5fb7 to a9b4c22 Compare December 18, 2023 16:23

camertron reviewed Dec 18, 2023

View reviewed changes

boardfish force-pushed the fix-appraisal-ci-boardfish branch from a9b4c22 to fd21097 Compare December 19, 2023 09:07

boardfish requested a review from camertron December 19, 2023 09:36

boardfish requested a review from BlakeWilliams as a code owner January 3, 2024 09:44

boardfish marked this pull request as draft January 3, 2024 09:45

boardfish force-pushed the fix-appraisal-ci-boardfish branch 4 times, most recently from 7146d23 to 6891bba Compare January 3, 2024 13:52

boardfish marked this pull request as ready for review January 3, 2024 15:29

joelhawksley reviewed Jan 3, 2024

View reviewed changes

camertron requested changes Jan 3, 2024

View reviewed changes

boardfish force-pushed the fix-appraisal-ci-boardfish branch 2 times, most recently from 07fa887 to beb9857 Compare January 4, 2024 09:20

boardfish force-pushed the fix-appraisal-ci-boardfish branch from beb9857 to 56cf867 Compare January 6, 2024 16:19

boardfish force-pushed the fix-appraisal-ci-boardfish branch from 56cf867 to d9c202c Compare January 6, 2024 16:20

boardfish changed the title ~~Fix appraisal in CI~~ Use appraisal gem in CI Jan 6, 2024

boardfish merged commit d76f5db into main Jan 6, 2024
40 checks passed

boardfish deleted the fix-appraisal-ci-boardfish branch January 6, 2024 16:31

This was referenced Jan 6, 2024

Exclude I18n reserved keys from html escaping #1953

Merged

Add support for using view components in rails engine #1951

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `appraisal` gem in CI #1308

Use `appraisal` gem in CI #1308

boardfish commented Mar 14, 2022 •

edited

Loading

Spone commented Mar 16, 2022 •

edited

Loading

boardfish commented Mar 16, 2022

camertron left a comment

camertron Mar 24, 2022

Spone commented Jun 2, 2022

joelhawksley left a comment

allmarkedup commented Sep 14, 2022

Spone commented Jan 18, 2023

nickcharlton commented Jan 27, 2023

Spone commented Jan 27, 2023

Spone Feb 6, 2023

Spone commented Feb 6, 2023

Spone commented Feb 23, 2023

joelhawksley commented Feb 23, 2023

joelhawksley commented Dec 13, 2023

boardfish commented Dec 14, 2023

joelhawksley commented Dec 14, 2023

boardfish commented Dec 14, 2023

camertron left a comment

camertron commented Jan 2, 2024

joelhawksley Jan 3, 2024 •

edited

Loading

boardfish Jan 4, 2024

camertron Jan 3, 2024

boardfish commented Jan 4, 2024 •

edited

Loading

joelhawksley commented Jan 4, 2024

boardfish commented Jan 6, 2024

Use appraisal gem in CI #1308

Use appraisal gem in CI #1308

Conversation

boardfish commented Mar 14, 2022 • edited Loading

Spone commented Mar 16, 2022 • edited Loading

boardfish commented Mar 16, 2022

camertron left a comment

Choose a reason for hiding this comment

camertron Mar 24, 2022

Choose a reason for hiding this comment

Spone commented Jun 2, 2022

joelhawksley left a comment

Choose a reason for hiding this comment

allmarkedup commented Sep 14, 2022

Spone commented Jan 18, 2023

nickcharlton commented Jan 27, 2023

Spone commented Jan 27, 2023

Spone Feb 6, 2023

Choose a reason for hiding this comment

Spone commented Feb 6, 2023

Spone commented Feb 23, 2023

joelhawksley commented Feb 23, 2023

joelhawksley commented Dec 13, 2023

boardfish commented Dec 14, 2023

joelhawksley commented Dec 14, 2023

boardfish commented Dec 14, 2023

camertron left a comment

Choose a reason for hiding this comment

camertron commented Jan 2, 2024

joelhawksley Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

boardfish Jan 4, 2024

Choose a reason for hiding this comment

camertron Jan 3, 2024

Choose a reason for hiding this comment

boardfish commented Jan 4, 2024 • edited Loading

joelhawksley commented Jan 4, 2024

boardfish commented Jan 6, 2024

Use `appraisal` gem in CI #1308

Use `appraisal` gem in CI #1308

boardfish commented Mar 14, 2022 •

edited

Loading

Spone commented Mar 16, 2022 •

edited

Loading

joelhawksley Jan 3, 2024 •

edited

Loading

boardfish commented Jan 4, 2024 •

edited

Loading