Reordering test code is unsafe and breaks tests

[p-datadog]

Be clear, concise and precise in your description of the problem.
Open an issue with a descriptive title and a summary in grammatically correct,
complete sentences.

Use the template below when reporting bugs. Please, make sure that
you're running the latest stable RuboCop RSpec and that the problem you're reporting
hasn't been reported (and potentially fixed) already.

Before filing the ticket you should replace all text above the horizontal
rule with your own words.

In the case of false positive or false negative, please add the
corresponding cop name.

---

Expected behavior

I expect rubocop to not break my tests.

Actual behavior

Rubocop attempted to reorder my test code, putting overriden let blocks above defaults, by suggesting the following change:

--- a/spec/datadog/core/crashtracking/component_spec.rb
+++ b/spec/datadog/core/crashtracking/component_spec.rb
@@ -224,18 +224,18 @@ RSpec.describe Datadog::Core::Crashtracking::Component, skip: !CrashtrackingHelp
 
       context 'via unix domain socket' do
         let(:temporary_directory) { Dir.mktmpdir }
-        let(:socket_path) { "#{temporary_directory}/rspec_unix_domain_socket" }
-        let(:unix_domain_socket) { UNIXServer.new(socket_path) } # Closing the socket is handled by webrick
-        define_http_server do |http_server|
-          http_server.listeners << unix_domain_socket
-          http_server.mount_proc('/', &server_proc)
-        end
         let(:http_server_options) do
           {
             DoNotListen: true,
           }
         end
         let(:agent_base_url) { "unix://#{socket_path}" }
+        let(:socket_path) { "#{temporary_directory}/rspec_unix_domain_socket" }
+        let(:unix_domain_socket) { UNIXServer.new(socket_path) } # Closing the socket is handled by webrick
+        define_http_server do |http_server|
+          http_server.listeners << unix_domain_socket
+          http_server.mount_proc('/', &server_proc)
+        end
 
         after do
           FileUtils.remove_entry(temporary_directory)

In the above snippet, define_http_server defines http_server_options that then is overridden by the test. Rubocop wants to put the define_http_server line after the override, thus making the override be replaced by the base implementation provided by the helper. This is wrong behavior and breaks the test.

Steps to reproduce the problem

Define a helper that defines let blocks, then override one of such let blocks in a test.

RuboCop RSpec version

big% be rubocop -V
1.71.2 (using Parser 3.3.7.4, rubocop-ast 1.44.0, analyzing as Ruby 2.6, running on ruby 3.4.1) [x86_64-linux]
  - rubocop-packaging 0.5.2
  - rubocop-performance 1.23.1
  - rubocop-rspec 2.31.0

[bquorning]

In the above snippet, define_http_server defines http_server_options

If I understand correctly, calling #define_http_server defines a let which you then want to override. And when RuboCop groups the let calls, things are called in the wrong order.

I think that in most cases, our cops still do the right thing. I think in order to not reorder code like your use case, we would need to do dynamic analysis. And RuboCop is limited to doing static analysis.

I recommend that you use inline or block comments to disable the cop that moves your lines with let calls.