[CI] Run tests for individual gems in GitHub Actions #569

jagthedrummer · 2023-09-20T19:31:07Z

In #550 I realized that I was making the same changes over and over to each individual gem and that instead we should probably ship some base controller/models in the main bullet_train gem. So this is an attempt at doing that.

jagthedrummer · 2023-11-15T18:20:13Z

OK, I finally got this all working, but this situation is a little ridiculous for a single PR:

There are a few main things that this PR does, and I'm going to try to split it into smaller PRs as best I can.

Adds some missing dependencies to .gemspec files. These dependencies end up getting resolved when we're installing things within the starter repo, but then some gem tests fail when we run them outside of the starter repo context.
~~Add some dependencies to various Gemfiles. These make it so that in dev/test each gem will use the peer version of the gems in the core repo.~~
Adds some "missing" files like Account::ApplicationController. These are files that we ship in the starter repo, and which various gems expect to be present. So when we try to test the gems outside of the starter repo we need to provide those files in some way. We could potentially put a copy of them in test/dummy for each gem, but that seems unwieldy. Instead I'm adding them directly to the gem that seems appropriate and then the copy in the starter repo will be as if that file has been ejected.
Some CI plumbing in the .github diretory
A few random things that I had to change here and there to get things to work. These are probably the changes most likely to cause problems or to be things that we don't want to do, and it would be much easier to review them if they're not mixed in with all this other stuff.

In the course of [trying to be able to run all of the tests for individual gems in CI](#569) I've discovered that we don't specify all of our dependencies fully. In the starter repo the problems don't appear because all of the things that we need end up being dependencies of something or other, but when we try to run tests for individual gems we'll get errors about unsatisfied dependencies. There are only a few gems where we needed to declare a missing dependency in the `.gemspec`. And those are the only thing in this PR that could have the chance to impact downsteam apps. All of the changes to `Gemfile` and `Gemfile.lock` in each gem won't impact gem consumers at all. They just come into play in local development and CI. The changes in each `Gemfile` ensure that when developing or testing these gems that they'll use their peer gems from inside the `core` repo insted of using whatever version happens to be latest.

jagthedrummer · 2023-11-15T23:03:58Z

I handled the .gemspec and Gemfile dependencies in #667 and #668. Then rebased this branch and things aren't quite as bad:

Tomorrow I'll see what I can do to split this up further. And I'm sure there are some bits that I need to clean up.

bullet_train-outgoing_webhooks/test/dummy/app/models/team.rb

jagthedrummer · 2023-11-16T18:26:40Z

bullet_train-api/app/controllers/api/v1/application_controller.rb

@@ -0,0 +1,3 @@
+class Api::V1::ApplicationController < ActionController::API
+  include Api::Controllers::Base
+end


This and the next couple of files are just providing a default implementation of files that we ship in the starter repo. We can think of the files that we ship in the starter repo as being "ejected" versions of these files. I added these in the gem itself so that we don't have to supply these files in test/dummy of every singe gem that ends up needing them.

I remember this being really annoying because the tests complained about the files not being there. This seems like a good option!

I think it would be helpful to provide a comment in these files briefly explaining the situation so in the future someone on the Bullet Train team doesn't come across it and mistake it for a "duplicated" file in the starter repository.

jagthedrummer · 2023-11-16T18:48:20Z

...rain-incoming_webhooks/app/controllers/webhooks/incoming/bullet_train_webhooks_controller.rb

@@ -1,6 +1,6 @@
 class Webhooks::Incoming::BulletTrainWebhooksController < Webhooks::Incoming::WebhooksController
  def create
-    Webhooks::Incoming::BulletTrainWebhook.create(data: JSON.parse(request.body.read)).process_async
+    Webhooks::Incoming::BulletTrainWebhook.create(data: JSON.parse(request.raw_post)).process_async


For some reason request.body.read is returning an empty string '' which then causes JSON.parse to throw an error. Using raw_post seems to work. At least for tests...

jagthedrummer · 2023-11-16T18:49:33Z

bullet_train-incoming_webhooks/config/models/roles.yml

This is a "default implementation" of something we ship with the starter repo. Putting it in this specific spot feels kinda wrong, but that's what I had to do to get tests to pass.

jagthedrummer · 2023-11-16T18:50:36Z

bullet_train-incoming_webhooks/test/dummy/config/database.yml

This and the next few files are just setting up the database for the test env of bullet_train-incoming_webhooks.

jagthedrummer · 2023-11-16T18:51:23Z

bullet_train-outgoing_webhooks/config/models/webhooks/outgoing/event_types.yml

Another "default implementation" that feels like it's not in quite the right place, but it had to be here for tests to work.

jagthedrummer · 2023-11-16T18:52:12Z

bullet_train-outgoing_webhooks/test/dummy/config/database.yml

This and the next few files are just setting up the database for the test env of bullet_train-outgoing_webhooks.

jagthedrummer · 2023-11-16T18:56:52Z

...ain-outgoing_webhooks/test/integration/api/a1/webhooks/outgoing/endpoints_controller_test.rb

@@ -3,6 +3,9 @@
 class Api::V1::Webhooks::Outgoing::EndpointsControllerTest < ActionDispatch::IntegrationTest
  setup do
    @controller = Api::V1::Webhooks::Outgoing::EndpointsController
+
+    @team = Team.first
+    @endpoint = @team.webhooks_outgoing_endpoints.first


Grabbing a couple of objects for reference later in the tests. Doing this since we can't (and shouldn't try to) rely on hard coded ids.

jagthedrummer · 2023-11-16T18:57:52Z

bullet_train-outgoing_webhooks/test/setup/active_record.rb

-    t.references :team
-    t.string :url, null: false
-    t.string :name, null: false
-    t.integer :event_type_ids, array: true, default: []


sqlite3 doesn't seem to support array: true in CI, so I just changed this gem to test against postgres which is what we target for deployment anyway.

jagthedrummer · 2023-11-16T19:04:11Z

...ain-outgoing_webhooks/test/integration/api/a1/webhooks/outgoing/endpoints_controller_test.rb

+
+    5.times do |n|
+      Webhooks::Outgoing::Endpoint.create! team_id: Team.first.id, name: "Generic name #{n}", url: "http://example.com/webhook-#{n}"
+    end


Moved these into the setup method so that they're created within the transaction for running tests. When they were outside of the setup method in the setup/active_record.rb file the records created would stay in the DB between test runs.

jagthedrummer · 2023-11-16T19:06:55Z

bullet_train-scope_validator/bin/rails

Added a missing binstub.

jagthedrummer · 2023-11-16T19:08:06Z

bullet_train-super_scaffolding/test/dummy/app/controllers/account/application_controller.rb

Removed this so that we use the default implementation from the core gems.

jagthedrummer · 2023-11-16T19:13:03Z

bullet_train/app/controllers/account/application_controller.rb

Added a default implementation of this file so that it doesn't need to be in test/dummy for each gem. The next few files are also default implementations.

jagthedrummer · 2023-11-16T19:55:52Z

bullet_train-api/test/dummy/app/controllers/application_controller.rb

I removed a bunch of */test/dummy/app/controllers/application_controller.rb */test/dummy/app/models/application_record.rb files so that gems will use the default implementation that we ship.

jagthedrummer · 2023-11-16T20:00:38Z

bullet_train/app/models/concerns/users/base.rb

@@ -2,6 +2,7 @@ module Users::Base
  extend ActiveSupport::Concern

  included do
+    extend Devise::Models


Explicitly including this so that things will work in tests when the rail ties don't do the thing.

jagthedrummer · 2023-11-16T20:04:04Z

bullet_train/lib/bullet_train/configuration.rb

@@ -24,7 +24,7 @@ def enable_bulk_invitations
      end

      def incoming_webhooks_parent_class_name
-        @@config&.incoming_webhooks_parent_class_name
+        @@config&.incoming_webhooks_parent_class_name || "ApplicationRecord"


Need to supply a default value here or we get a failure like this:

Error: Webhooks::Incoming::BulletTrainWebhooksControllerTest#test_should_get_incoming_webhook: NoMethodError: undefined method `constantize' for nil:NilClass /Users/jgreen/projects/bullet-train-co/bullet_train-core/bullet_train-incoming_webhooks/app/models/webhooks/incoming/bullet_train_webhook.rb:1:in `<top (required)>' <internal:/Users/jgreen/.asdf/installs/ruby/3.2.2/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require' <internal:/Users/jgreen/.asdf/installs/ruby/3.2.2/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require' /Users/jgreen/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/zeitwerk-2.6.12/lib/zeitwerk/kernel.rb:30:in `require' /Users/jgreen/projects/bullet-train-co/bullet_train-core/bullet_train-incoming_webhooks/app/controllers/webhooks/incoming/bullet_train_webhooks_controller.rb:3:in `create'

This reverts commit 82123cb.

jagthedrummer force-pushed the jeremy/github-gem-tests-redux branch 2 times, most recently from 6d2872e to 93be509 Compare November 15, 2023 17:40

jagthedrummer mentioned this pull request Nov 15, 2023

[CI] Be more explicit about some dependencies #667

Merged

jagthedrummer force-pushed the jeremy/github-gem-tests-redux branch 2 times, most recently from dc3ea6f to 7c94fc5 Compare November 15, 2023 22:06

jagthedrummer force-pushed the jeremy/github-gem-tests-redux branch from 7c94fc5 to fbb3451 Compare November 16, 2023 16:18

jagthedrummer mentioned this pull request Nov 16, 2023

Run tests for individual gems in GitHub Actions #550

Closed

jagthedrummer force-pushed the jeremy/github-gem-tests-redux branch from eb9ae05 to 926524d Compare November 16, 2023 17:27

jagthedrummer commented Nov 16, 2023

View reviewed changes

bullet_train-outgoing_webhooks/test/dummy/app/models/team.rb Outdated Show resolved Hide resolved

jagthedrummer commented Nov 16, 2023

View reviewed changes

jagthedrummer force-pushed the jeremy/github-gem-tests-redux branch from 7eb90ed to 8eead89 Compare November 16, 2023 18:45

jagthedrummer commented Nov 16, 2023

View reviewed changes

bullet_train-scope_validator/bin/rails Outdated

Copy link

Contributor Author

jagthedrummer Nov 16, 2023

Choose a reason for hiding this comment

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

Added a missing binstub.

jagthedrummer commented Nov 16, 2023

View reviewed changes

jagthedrummer marked this pull request as ready for review November 16, 2023 20:53

jagthedrummer added 26 commits November 22, 2023 09:32

more clean up

83d7666

make a note

c1ac0da

do we need this?

febc916

clean up

02ff7dd

add back a file that we maybe don't need to delete

842769b

remove extra line

345333c

clean up

ca4ca95

more clean up

e4ae34f

more clean up

69115f2

moar clean up

6c2e402

clean up

347f6d0

redundant application_model

ec8705f

remove application_record.rb from all dummy apps

0560920

Remove application_controller.rb from all dummy apps

988b07c

apparently we need this

eaa9e9a

I guess we need this

9dab997

clean up

3ccec3e

clean up

bd605ae

clean up

834aa91

will caching work now

f3d205a

clean up

a2f872c

moar clean up

915526b

maybe we should include this by default

c77adee

Revert "maybe we should include this by default"

b9e266a

This reverts commit 82123cb.

Make some notes on new 'default implementation' files

b696dc8

one more

672fa00

jagthedrummer force-pushed the jeremy/github-gem-tests-redux branch from 5dfbe87 to 672fa00 Compare November 22, 2023 15:32

emojis are v importante

f4a1525

jagthedrummer merged commit fce0791 into main Nov 22, 2023
18 checks passed

jagthedrummer deleted the jeremy/github-gem-tests-redux branch November 22, 2023 15:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CI] Run tests for individual gems in GitHub Actions #569

[CI] Run tests for individual gems in GitHub Actions #569

jagthedrummer commented Sep 20, 2023

jagthedrummer commented Nov 15, 2023 •

edited

Loading

jagthedrummer commented Nov 15, 2023

jagthedrummer Nov 16, 2023

gazayas Nov 21, 2023

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023 •

edited

Loading

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023

jagthedrummer Nov 16, 2023

[CI] Run tests for individual gems in GitHub Actions #569

[CI] Run tests for individual gems in GitHub Actions #569

Conversation

jagthedrummer commented Sep 20, 2023

jagthedrummer commented Nov 15, 2023 • edited Loading

jagthedrummer commented Nov 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jagthedrummer Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jagthedrummer commented Nov 15, 2023 •

edited

Loading

jagthedrummer Nov 16, 2023 •

edited

Loading