Remove unused attr_writer

[cafedomancer]

Description

from #2918 (comment).

Your checklist for this pull request

• I have reviewed the guidelines for contributing to this repository.
• I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
• My pull request is 100 lines added/removed or less so that it can be easily reviewed.
• If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
• If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
• I have updated the documentation accordingly.
• All new and existing tests passed, including Rubocop.

[MSP-Greg]

@cafedomancer

Thanks for working on this.

In the comment, I mentioned "(tests may need changes)". Ok, that was not true. I should have said "tests will require changes, and there may be several files that require small changes to several tests.".

Tests that start the server in setup may require fixes like instance_variable_set.

Below is a quick patch for test_persistent.rb.

test_persistent.rb patch

diff --git a/test/test_persistent.rb b/test/test_persistent.rb
index be90d09b..863c4d6d 100644
--- a/test/test_persistent.rb
+++ b/test/test_persistent.rb
@@ -1,4 +1,5 @@
 require_relative "helper"
+require "puma/log_writer"
 
 class TestPersistent < Minitest::Test
 
@@ -23,9 +24,9 @@ class TestPersistent < Minitest::Test
       [status, @headers, @body]
     end
 
-    @server = Puma::Server.new @simple
+    opts = {min_threads: 1, max_threads: 1}
+    @server = Puma::Server.new @simple, Puma::LogWriter.stdio, Puma::Events.new, opts
     @port = (@server.add_tcp_listener HOST, 0).addr[1]
-    @server.max_threads = 1
     @server.run
     sleep 0.15 if Puma.jruby?
     @client = TCPSocket.new HOST, @port
@@ -156,7 +157,7 @@ class TestPersistent < Minitest::Test
   end
 
   def test_persistent_timeout
-    @server.persistent_timeout = 1
+    @server.instance_variable_set(:@persistent_timeout, 1)
     @client << @valid_request
     sz = @body[0].size.to_s
 
@@ -193,7 +194,7 @@ class TestPersistent < Minitest::Test
 
 
   def test_two_requests_in_one_chunk
-    @server.persistent_timeout = 3
+    @server.instance_variable_set(:@persistent_timeout, 3)
 
     req = @valid_request.to_s
     req += "GET /second HTTP/1.1\r\nHost: test.com\r\nContent-Type: text/plain\r\n\r\n"
@@ -210,7 +211,7 @@ class TestPersistent < Minitest::Test
   end
 
   def test_second_request_not_in_first_req_body
-    @server.persistent_timeout = 3
+    @server.instance_variable_set(:@persistent_timeout, 3)
 
     req = @valid_request.to_s
     req += "GET /second HTTP/1.1\r\nHost: test.com\r\nContent-Type: text/plain\r\n\r\n"

[cafedomancer]

@MSP-Greg Thanks for sharing a patch. I just applied it and fixed some tests.

[nateberkopec]

Later, we should rewrite the tests so we can pass this in as an option when we create the server object, rather than have to change the instance variable.

[dentarg]

The real motivation behind removing these methods wasn't spelled out here so I dug it up, see these comments from Nate and Greg:

• Consolidate option handling in Server, Server small refactors, doc changes #2373 (comment)
• Consolidate option handling in Server, Server small refactors, doc changes #2373 (comment)