Consolidate option handling in Server, Server small refactors, doc changes

[MSP-Greg]

Description

In normal use, Server is passed an options hash in its initialize method. Some code is outside of Server, using attr_accessors, some of which may have been created for CI, etc. They also display in docs, which isn't needed.

1. server.rb - consolidate option handling, remove attr_accessor statements, change other lib files as needed.

2. server.rb - Server had a method defined in class << self (tcp_cork_supported?), and also a method defined as self.current. Move current into class << self block.

3. runner.rb - remove development? & test?, which are only used to set a value based on options in Server.

4. Change test files to use Server#instance_variable_get

5. Add YARD # @!attribute tags to methods that are actually attributes (no parameters, getter, setter, or predicate)

For those interested in seeing Puma docs:
current gem https://puma.io/puma
master https://msp-greg.github.io/puma/

Your checklist for this pull request

• I have reviewed the guidelines for contributing to this repository.
• I have added an entry to History.md if this PR fixes a bug or adds a feature. If it doesn't need an entry to HISTORY.md, I have added [changelog skip] or [ci skip] to the pull request title.
• I have added 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.

[dentarg]

1. remove attr_accessor statements

Could be considered breaking changes if someone is working programmatically against Puma::Server?

[MSP-Greg]

@dentarg

Could be considered breaking changes

Yes, that is true. But, given that Puma is essentially a CLI app, I don't think the idea of 'what is the public API' has been given much attention. Puma is not a young app, and over time, proper encapsulation of objects often leaks.

From my perspective, when I see attr_accessor or attr_writer, I wonder why write access is given. In the case of Server, there really isn't any reason for write access to exist.

I suppose I can revert all the attribute changes, but they just clutter up the real methods of the class. Maybe we should implement 'deprecated' warnings? I don't know.

I think that the more the API is cleaned up, the easier it is for new contributors to understand how Puma works...

[nateberkopec]

I'm OK with removing write access where that write access didn't do anything (e.g. we never recheck the value after server start, so writing to the variable is a no-op).

I don't think we can remove reads without a deprecation policy anymore.

[MSP-Greg]

I did check what was passed to the various hooks and the plugin, I'll add back in reader code.

EDIT: Rebased and added attr_reader statements...

[MSP-Greg]

One issue. In server.rb:83, I changed @max_threads from 16 to (Puma.mri? ? 5 : 16), which aligns it with the new default.

[dentarg]

1. remove attr_accessor statements

Could be considered breaking changes if someone is working programmatically against Puma::Server?

Sorry, but this time I'm just gonna: #2388 😉

[MSP-Greg]

I've got an update that adds writers to this commit and marks them deprecated. Haven't pushed yet. It was mostly to clean up the following:

class Server
  attr_accessor :min_threads, :max_threads
  
  def initialize(app, events=Events.stdio, options={})
    # code
  end
end

class Runner
  def start_server
    server = Server.new(app, events, opts)
    server.min_threads = opts[:min_threads]
    server.max_threads = opts[:max_threads]
    # code
  end
end

So, we're passing opts to Server.new, then setting two values in start_server immediately after using opts. Breaks encapsulation, and adds unneeded code to start_server...