New Test Framework?

[MSP-Greg]

PR #3179 adds two files to the test/helpers folder. These files are the first files from a new test framework. They may require non-breaking changes when the 'server side' code is added to the framework.

Below I've listed some goals of the new framework. Looking for feedback about the listed goals, and whether new goals should be added.

If people think this is a worthwhile task, I'll start by committing the framework files, then in subsequent PR's update individual test files to use it.

Goals

1. The framework should support the following socket types: tcp4, tcp6, ssl4, ssl6, unix, and aunix. These should be available for both bind and control. Centralized socket code should also make it easier to support QUIC and HTTP/3 if and when needed.

2. Since we are running CI tests parallel (with threads), any use of blocking operations should be minimized. I think many of the intermittent failures in current CI are caused by blocking test code.

3. The framework should make tests portable between files. Currently, a lot of files have their own code for client sockets and/or server creation, making it tedious to reorganize files. Note that some test files have just a handful of tests, others have dozens. I hope that reorganizing the test files will make it easier for contributors to find tests that are similar to tests they'd like to add.

4. Make test code agnostic as to the bind and/or control socket type. Or, make it trivial to change the socket type either in an individual test, a whole file, or possibly the whole test suite. One example would be changing all tests from tcp4 to tcp6 (IPv4 to IPv6). Changing all 'port' based tests from tcp4 to ssl6 might involve some careful code, but should be possible.

5. Discourage the use of native sockets (See 2 and 4). Native code that works locally may cause intermittent failures when run in parallel CI. Also, items like closing client sockets, removing UNIXSocket files, forcing server shutdown, etc are much better handled by a test framework.

6. Use Minitest's before_shutdown and after_teardown. This helps with test files using setup and teardown and not properly including super in those methods.

7. Properly document all the helper files.

File Layout

helpers
└─ test_puma.rb
   test_puma
   └─ puma_socket.rb
      server_base.rb
      server_in_process.rb
      server_spawn.rb
      server_cli.rb

File contents

• test_puma.rb - TestPuma, module, main namespace, contains constants related to sockets
• puma_socket.rb - PumaSocket, module, contains client socket methods
• server_base.rb - ServerBase, subclass of ::Minitest::Test, contains common code for subclasses
• server_in_process.rb - ServerInProcess, subclass of ServerBase, creates Puma::Server instances
• server_spawn.rb - ServerSpawn, subclass of ServerBase, creates Puma instances using spawn
• server_cli.rb - ServerCLI, subclass of ServerBase, creates Puma instances using Puma::CLI.new(...).run

Questions

8. Most of the current test suite client socket code expects a fully constructed string for the request. Should the test framework allow construction of a request with headers, body, etc? Similar to net/http...

[dentarg]

All this sounds very good and sensible to me. Thank you for working on it @MSP-Greg 💪

What do you think of this name change? ServerInProc -> ServerInProcess, server_inproc.rb -> server_in_process.rb

[MSP-Greg]

What do you think of this name change

Ok by me, fixed above.

[MSP-Greg]

@dentarg, I added a 'Questions' section above, if you have any strong feelings about, please respond.

One tricky thing with frameworks is deciding how many edge cases can be supported without making the code a lot more complex/messy.

One example, which I think can be done easily, is sending a socket that is different from the server's protocol. An example currently used in one test is sending a TCP request to an SSL server. I'm sure there are more (and worse) examples...

[nateberkopec]

Wow! An excellent proposal. We're very lucky to have you involved, Greg.

I think especially this will help with new contributions. I have felt the pain point of number 3 even as a maintainer.

Re:

Most of the current test suite client socket code expects a fully constructed string for the request. Should the test framework allow construction of a request with headers, body, etc?

Some sort of helper for this would be great. I think most contributors are not used to thinking of HTTP requests as strings but are quite used to the idea of a "request with headers and optional body".

[MSP-Greg]

Some sort of helper for this would be great.

Agreed. I think it can be a separate class, so it can be independent of the initial framework. I'll check how net/http assembles a request string...

I haven't mentioned why I'd like to change 'integration' to 'spawn'. Currently, all integration tests use IO.popen. IO.popen is essentially a derivative of spawn, but it doesn't let one capture STDOUT and STDERR separately. The framework will use spawn to start Puma instances, as it allows us to separate them.

[MSP-Greg]

I'm going to start work on the 'server' files, along with converting existing test files to use them. Once it's far enough along, one PR with the framework server files, then additional PR's with each test file conversion...

[MSP-Greg]

Update: Working on converting files to use PumaSocket for all client request processing. Doing so is also reminding me of all the needs for the new classes handling server creation.

All of the server classes will have a keyword argument bind_type, and if applicable, control_type. These refer to the socket type used.
It can also be set in the setup test method in each test file.

These will set variables that PumaSocket understands, so test code won't ever have a need for host/port, or UNIX socket file paths. Also, this means that there shouldn't be a need to use native sockets or net/http in any of test files...

JFYI, in master, TestPumaServer take about 20 seconds on my ten core system. The update using PumaSocket takes about 3. That's an exception, most files show much smaller decreases...