Upgrade test coverage from 24.6% to 94.12%

[mmeest]

This PR improves test coverage and refactors the test suite for better maintainability by splitting a large test file into smaller, focused test files.

Changes

Test Coverage Improvements

• Added additional test cases to unicode_validator_test.rb to improve coverage from 45.45% to 100%
  • Added tests for multi-byte hex-escaped UTF-8 strings
  • Added tests for ArgumentError handling
  • Added tests for hex sequence unescaping logic

Test Structure Refactoring

• Split test/whois_server_test.rb (449 lines, 70+ test methods) into three focused test files:
  • test/unit/whois_server_core_test.rb - Domain validation and regexp tests (6 tests)
  • test/unit/whois_server_methods_test.rb - Individual method tests (25 tests)
  • test/integration/whois_server_integration_test.rb - Integration flow tests (13 tests)
• This improves maintainability and makes it easier to locate specific test categories

Configuration

• Fixed SimpleCov initialization to start at the top of test_helper.rb for accurate coverage tracking

Test Results

• ✅ All 108 tests passing
• ✅ Coverage: 200/205 lines covered (97.56%)
• ✅ 0 failures, 0 errors, 0 skips

[OlegPhenomenon]

You added a stub to the invalid_data? method so it always returns true. Then you send data that’s obviously invalid? What if I decide to send valid data here: will the test fail, or will it just claim my valid data is invalid too?

[mmeest]

Obviously this test would return same result despite sent data as it is focused on code behaviour in negative scenario. But for better handling test is now rewritten.

[OlegPhenomenon]

What exactly is this test checking? You’ve mocked so many methods that I can’t tell what we’re actually testing here.

[mmeest]

It was testing on positive scenario. It is rewritten: test_receive_data_returns_valid_whois_body

[OlegPhenomenon]

And what are we testing in this one? Is it any different from the previous test, apart from the domain name?

[mmeest]

It was very similar. And now it is removed to reduce duplication.

[OlegPhenomenon]

process_whois_request is a private method. It’s generally considered bad practice to test a private method directly, except in rare cases. You should prepare test data that triggers this method indirectly, so we can verify how it behaves through its effects. A private method is meant to be hidden, no one outside the class should know about it. It’s a black box.

[mmeest]

Testing private methods is removed.

[OlegPhenomenon]

Same thing here. The issue was that when we ran code like printf 'täpiline.ee\r\n' | nc localhost 43, the WHOIS service would crash. That’s why we updated the process_whois_request method to add extra checks: specifically for cases when the invalid_data? method in the public scope doesn’t see anything suspicious.

Honestly, I’d slightly restructure and refactor this, but the problem is that if I move some of the logic from private methods up to a higher-level abstraction, for example, from process_whois_request into receive_data, the tests will start failing. And that’s exactly the point I’m making: you shouldn’t be testing private methods directly. Public methods should be the ones calling them as needed, depending on the data and conditions.

[mmeest]

All testing on focused for private methods have been removed.

[OlegPhenomenon]

what differents between this test and test_receive_data_full_flow?

[mmeest]

Duplication was a bit heavy in these tests. Now the duplication have been reduced by removing similar tests and rewritten test logic.