Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🧪 Add experimental new FakeServer for tests #157

Merged
merged 2 commits into from
Jul 17, 2023

Conversation

nevans
Copy link
Collaborator

@nevans nevans commented Jul 17, 2023

This was written for several reasons:

  • To improve yields_in_test_server_thread and clean up tests using it
  • To support new tests for ✨ Cache server capabilities and add #capable?(name) #156
  • To speed up adding test coverage for commands, both new and old
  • Because I kept messing up OpenSSL::SSL::SSLSocket#accept, even
    though I copied and pasted from imaps_test and starttls_test. 😳
  • To support testing of other libraries and applications that use Net::IMAP.

I will (probably) eventually update most of the tests in test_imap.rb to use FakeServer. The version here is able to support the capabilities tests and several more existing tests. To quote the rdoc:

NOTE: API is experimental and may change without deprecation or warning.

FakeServer is simple fake IMAP server that is used for testing Net::IMAP. It contains simple implementations of many IMAP commands and allows customization of server responses. This allow tests to assume a more-or-less "normal" IMAP server implementation, so as to focus on what's important for what's being tested without needing to fuss over the details of a TCPServer script.

Although the API is not (yet) stable, Net::IMAP::FakeServer is also intended to be useful for testing libraries and applications which themselves use Net::IMAP.

Limitations

FakeServer cannot be a complete replacement for exploratory testing or integration testing with actual IMAP servers. Simple default behaviors will be provided for many commands, and tests may simulate specific server responses by assigning handlers (using #on).

And FakeServer is significantly more complex than simply creating a socket IO script in a separate thread. This complexity may obscure the focus of some tests or make it more difficult to debug them. Use with discretion.

Currently, the server will shutdown after a single connection has been accepted and closed. This may change in the future, but only if tests can be simplified or made significantly faster by allowing multiple connections to the same TCPServer.

The following tests in test_imap.rb have been updated to use FakeServer:

  • test_clear_responses
  • test_close
  • test_enable
  • test_responses
  • test_uid_expunge
  • test_uidplus_responses
  • test_unselect

All of the tests that used yields_in_test_server_thread were updated, so that method was deleted too. It has effectively been replaced by with_fake_server.

This was written for several reasons:
* To improve `yields_in_test_server_thread` and clean up tests using it
* To support new tests for the new cached capabilities methods
* Because I kept messing up `OpenSSL::SSL::SSLSocket#accept`, even
  though I copied and pasted from `imaps_test` and `starttls_test`. 😳
* To support testing of other libraries and applications that use
  Net::IMAP.

I will (probably) eventually update most of the tests in `test_imap.rb`
to use FakeServer.  The version here is able to support the capabilities
tests and several more existing tests.  See the rdoc on FakeServer for
more info.
* test_clear_responses
* test_close
* test_enable
* test_responses
* test_uid_expunge
* test_uidplus_responses
* test_unselect

All of the tests that used `yields_in_test_server_thread` were updated,
so that method was deleted too.
@nevans nevans changed the title 🧪 Add new FakeServer for tests 🧪 Add experimental new FakeServer for tests Jul 17, 2023
@nevans nevans merged commit b21e845 into ruby:master Jul 17, 2023
@nevans nevans requested review from hsbt and shugo July 17, 2023 13:13
@nevans
Copy link
Collaborator Author

nevans commented Jul 17, 2023

@hsbt, @shugo I wanted to check in and make sure you're okay with this. Although I tried to keep it simple, readable, and relatively easy for someone new to use, it's significantly more complex than the simple socket.gets/socket.print scripts we were using before.

This PR added over 500 lines of code in order to save only ~50 lines of test code. I could've saved a few more lines of code, but I did add new assertions and split some test methods into multiple tests. Nevertheless, it's still an order of magnitude difference! As I continue to convert most of the existing tests and add new ones, that savings should grow.

I'm not fully satisfied with the internal structure or the API... but since I'm already using for a couple of future PRs, I wanted to add it in its current form, rather than delay until I was satisified with it... that day may never come!

My hope is that this allows the tests to focus on what matters, and thus be more approachable for us and for newcomers to read, debug, and write. But I can only answer that question for myself. After I've used it in more places and the API stabilizes I would like to either move it into lib/net/imap/test/fake_server.rb or its own gem, so it can be more easily used by other libraries and applications. What do you think?

@nevans nevans deleted the test-fake-server branch July 26, 2023 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant