Skip to content

Reinstantiate: Detect incompatible use of `Struct.new` in specs

Peter Leitzen requested to merge pl-struct-with-args-reinstantiated into master

What does this MR do and why?

This MR reinstantiate !164777 (merged) which was revert in !165668 (merged) due to master:broken gitlab-org/quality/engineering-productivity/master-broken-incidents#8370 (closed).

The root cause for the master broken was fixed by making sure that the specs passes on Ruby 3.1 as well:

asdf local ruby 3.1.5
bin/rspec spec/support_specs/struct_with_kwargs_spec.rb
10 examples, 0 failures

MR description from !164777 (merged)

This MR patches Struct.new in specs when keyword_init is missing and a Hash passed.

Reason: Since Ruby 3.2 Struct supports kwargs by default so keyword_init: true is no longer required. We are running Ruby 3.2 in MRs and Ruby 3.1 on master so occasionally master is broken due to this incompatibility.

This slows down creation of Struct objects in specs by factor 3.

Closes #474743 (closed).

Refs !161677 (comment 2086365521)

How to set up and validate locally

  1. Apply patch from !161677 (for commit !161677 (64e0e441))
    • Via: curl -sG https://gitlab.com/gitlab-org/gitlab/-/merge_requests/161677.patch | git apply
  2. Run a spec which uses incompatible Struct use
    • bin/rspec spec/requests/api/repositories_spec.rb:197 - to use RepoHelpers#sample_blob
  3. See the failure caught by StructWithKwargs:
Failures:

  1) API::Repositories GET /projects/:id/repository/blobs/:sha when authenticated as a developer behaves like repository blob returns blob attributes as json
     Failure/Error:
               raise <<~MESSAGE
                 Passing only keyword arguments to #{self.class}#initialize will behave differently from Ruby 3.2. Please pass `keyword_init: true` to `Struct` constructor or use a Hash literal like .new({k: v}) instead of .new(k: v).
               MESSAGE

     RuntimeError:
       Passing only keyword arguments to RepoHelpers::BlobStruct#initialize will behave differently from Ruby 3.2. Please pass `keyword_init: true` to `Struct` constructor or use a Hash literal like .new({k: v}) instead of .new(k: v).
  1. Fix RepoHelpers#BlobStruct by adding keyword_init: true
  2. Re-ran the spec

Performance

This patch slows down the creation of Struct objects (not classes!) in specs by factor 3:

$ STRUCT_WITH_KWARGS=1 ruby foo.rb
Warming up --------------------------------------
          with Y.new   249.948k i/100ms
Calculating -------------------------------------
          with Y.new      2.515M (± 4.2%) i/s -     12.747M in   5.078006s

Comparison:
       without Y.new:  7824852.3 i/s
          with Y.new:  2515205.9 i/s - 3.11x  slower
The script
# frozen_string_literal: true

require 'benchmark/ips'

require_relative 'spec/support/struct_with_kwargs' if ENV['STRUCT_WITH_KWARGS']

label = ENV['STRUCT_WITH_KWARGS'] ? 'with' : 'without'

X = Struct.new(:a)

Benchmark.ips do |x|
  x.report "#{label} Y.new" do
    X.new(1)
  end

  x.save! 'tmp_results'
  x.compare!
end

Run twice:

ruby foo.rb
STRUCT_WITH_KWARGS=1 ruby foo.rb
Edited by Peter Leitzen

Merge request reports

Loading