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

tapioca dsl --verify succeeds when it should fail #1783

Open
bdewater-thatch opened this issue Feb 1, 2024 · 4 comments
Open

tapioca dsl --verify succeeds when it should fail #1783

bdewater-thatch opened this issue Feb 1, 2024 · 4 comments

Comments

@bdewater-thatch
Copy link
Contributor

bdewater-thatch commented Feb 1, 2024

We run Tapioca verify commands in CI, but with some regularity notice that RBI changes are missed for DSLs when we run the dsl generation locally. Our ci.yml step on GitHub actions (Linux):

- name: Ensure RBI files are up-to-date
  run: |
    bin/tapioca gems --verify
    bin/tapioca dsl --verify

output:

Run bin/tapioca gems --verify
  bin/tapioca gems --verify
  bin/tapioca dsl --verify
  shell: /usr/bin/bash -e {0}
  env:
    ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY: ***
    ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY: ***
    ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT: ***
    DISABLE_AWS_SECRETS: true
    CACHE_KEY: ***
Checking for out-of-date RBIs...

Nothing to do, all RBIs are up-to-date.
Loading DSL extension classes... Done
Loading Rails application... Done
Loading DSL compiler classes... Done
Checking for out-of-date RBIs...

Nothing to do, all RBIs are up-to-date.

Locally running bin/tapioca dsl --verify on a macOS machine does show me the error I was expecting to see:

bdewater@thatch-bart thatch % bin/tapioca dsl --verify
Loading DSL extension classes... Done
Loading Rails application... Done
Loading DSL compiler classes... Done
Checking for out-of-date RBIs...


RBI files are out-of-date. In your development environment, please run:
  `bin/tapioca dsl`
Once it is complete, be sure to commit and push any changes
If you don't observe any changes after running the command locally, ensure your database is in a good
state e.g. run `bin/rails db:reset`

Reason:
  File(s) added:
  - sorbet/rbi/dsl/anonymized_view_component.rbi
  File(s) changed:
  - sorbet/rbi/dsl/employee_mailer.rbi
  - sorbet/rbi/dsl/member_mailer.rbi

The diff of anonymized_view_component.rbi:

diff --git sorbet/rbi/dsl/anonymized_view_component.rbi sorbet/rbi/dsl/anonymized_view_component.rbi
new file mode 100644
index 00000000..83d94d59
--- /dev/null
+++ sorbet/rbi/dsl/anonymized_view_component.rbi
@@ -0,0 +1,10 @@
+# typed: true
+
+# DO NOT EDIT MANUALLY
+# This is an autogenerated file for dynamic methods in `AnonymizedViewComponent`.
+# Please instead update this file by running `bin/tapioca dsl AnonymizedViewComponent`.
+
+class AnonymizedViewComponent
+  include GeneratedUrlHelpersModule
+  include GeneratedPathHelpersModule
+end

The diff of the mailers show a method being moved:

diff --git sorbet/rbi/dsl/employee_mailer.rbi sorbet/rbi/dsl/employee_mailer.rbi
index 6d4b96a3..a7afa834 100644
--- sorbet/rbi/dsl/employee_mailer.rbi
+++ sorbet/rbi/dsl/employee_mailer.rbi
@@ -47,8 +47,5 @@ class EmployeeMailer
 
     sig { params(opt_in_required: T::Boolean).returns(::ActionMailer::MessageDelivery) }
     def qme_email(opt_in_required: T.unsafe(nil)); end
-
-    sig { returns(::ActionMailer::MessageDelivery) }
-    def tax_forms_available; end
   end
 end
diff --git sorbet/rbi/dsl/member_mailer.rbi sorbet/rbi/dsl/member_mailer.rbi
index d3e03791..f0e6dc4d 100644
--- sorbet/rbi/dsl/member_mailer.rbi
+++ sorbet/rbi/dsl/member_mailer.rbi
@@ -14,5 +14,8 @@ class MemberMailer
 
     sig { returns(::ActionMailer::MessageDelivery) }
     def plan_selection_confirmation_email; end
+
+    sig { returns(::ActionMailer::MessageDelivery) }
+    def tax_forms_available; end
   end
 end

As a workaround I can change our CI step to the following:

- name: Ensure RBI files are up-to-date
  run: |
    bin/tapioca gems
    bin/tapioca dsl
    git add -N .
    git diff --exit-code

git add -N . is to make sure new ViewComponent RBI is picked up by git diff as well, otherwise only the mailer change would show up.

... but I'd love to understand how to further debug this case and get this bug fixed.

@KaanOzkan
Copy link
Contributor

KaanOzkan commented Feb 5, 2024

My first thought is making sure the RAILS_ENV is consistent during both runs.

If you can't reproduce CI behaviour locally you'd have to ssh in and try to debug tapioca internals within that session. For the mailer issue I'd think action_methods_for_constant returns a different set of methods. You can look at the application code, see if anything funny is happening with tax_forms_available method definition.

For view_component, my guess would be on the gather_constants step of the compiler. More specifically the includes_helper? calls. Locally those are probably returning false. The ancestry of AnonymizedViewComponent could be different or potentially not being loaded at all.

I hope these give some ideas. It'd be easier if you could add a breakpoint to tapioca compilers and run it on CI, inspect relevant variables.

@bdewater-thatch
Copy link
Contributor Author

Had a little bit of spare time today to try and rule out the obvious:

  • bin/rails runner 'puts Rails.env' outputs development in CI as expected. Same for puts ENV["RAILS_ENV"]
  • The mailer is quite straightforward, a mail call using ActionMailer::Parameterized params, there is no metaprogramming happening.

Maybe related: the output of bin/tapioca dsl includes the line identical sorbet/rbi/dsl/employee_mailer.rbi but then git diff does show a change 🤔 I don't have the exact revision of our code handy of when I made the report, now the view component RBI doesn't show up. I'll try SSHing into the GH CI container next.

@andyw8
Copy link
Contributor

andyw8 commented Feb 21, 2024

Do you skip any Bundler groups in CI? e.g. with BUNDLE_WITHOUT.

@bdewater-thatch
Copy link
Contributor Author

No, we're using ruby/setup-ruby@v1 with bundler-cache: true and autodetected Ruby version (3.3) and are not setting it otherwise either.

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

No branches or pull requests

3 participants