Skip to content

Mock puma constant explicitly

João Alexandre Cunha requested to merge fix-puma-constant into master

What does this MR do and why?

Mock puma constant explicitly

This fixes a broken master: Thursday 2024-06-13 20:40 UTC - `gitlab-org/git... (gitlab-org/quality/engineering-productivity/master-broken-incidents#6851 - closed)

Some tests expect the Puma constant to not have been loaded, but for some reason, only in CI, the Puma constant is being loaded. There are 2 theories:

  • Either the stub_const('::Puma::Server', double) is leaking in between test examples.
  • Or the Puma server is actually running in the environment, which might have been caused ba a :js, type: :feature spec that ran before these tests and did not properly close the puma server.

This does not fix the above root causes, but it guarantees that the lingering constant won't interfere with the other tests that would otherwise expect that it is not set.

Simulating the failure scenario locally

There are 2 ways I was able to simulate the same failures. Without the fix from this MR, try one of the two options:

Option 1 - Pretend to be a Capybara test

Pretend this is a capybara test, so that Capybara will start the Puma server before the tests.

- RSpec.describe Gitlab::Runtime, feature_category: :cloud_connector do
+ RSpec.describe Gitlab::Runtime, :js, feature_category: :cloud_connector, type: :feature do

These will fail the tests the same as the master broken issue. I was hoping I could simulate this by just calling 2 tests together. One which is a Capybara test, and the runtime test. But calling one after the other does not fail them locally. So, we'd need to run them in parallel. I'm wondering if in CI, the Puma server takes longer to shut down, does it causes this behaviour.

Option 2 - Stub the Puma constant

For instance:

context "rails runner" do
    before do
+     stub_const('::Puma::Server', double)
      stub_const('::Rails::Command::RunnerCommand', double('::Rails::Command::RunnerCommand'))
    end

I also don't understand how this constant stub would leak in CI. I'm thinking that likely a parallel, perhaps dangling puma server from option 1 is really what's going on.

Other possibly better solutions

These are worth exploring, but they require more investigation and work, which I couldn't figure out quickly.

A better solution might involve:

  • Guaranteeing the puma server is properly shutdown before running runtime specs.
  • Perhaps isolate this spec from Capybara tests.
  • Guaranteeing that the stubbed constants are indeed reset in the beginning of the tests.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

Edited by João Alexandre Cunha

Merge request reports