DefaultDnsClientTest enhancements
- Dominant language
- Java
- Stars
- 1k
- Forks
- 229
- Avg merge
- 23h 23m
- Merged PRs (30d)
- 16
Description
`DefaultDnsClientTest` is challenging for a few reasons and could use an overhaul. Here are some of the areas for improvement:
- Uses `ServiceDiscovererTestSubscriber` which predates our asynchronous source testing utilities (e.g. `TestPublisher`, `TestCollectingPublisherSubscriber`, etc..). we should utilize the general testing utilities and verify each expected result.
- `TestRecordStore` has two data structures (`recordsToReturnByDomain` and `defaultRecordsByDomain`). The `getRecords` method removes the first entry from `recordsToReturnByDomain` upon resolution, and falls-back to `defaultRecordsByDomain` if the `recordsToReturnByDomain` lookup fails. This "remove first entry" makes it difficult to write tests and control events. We should consider making this controllable in a sequential fashion in the test execution if possible.
- `TestDnsServer` has a `DelegateRecordStore` class and a `store` method to change the `RecordStore` which is currently not used.
Contributor guide
Research direction
Start with DefaultDnsClientTest and read the existing use of ServiceDiscovererTestSubscriber, then compare it with the mentioned TestPublisher and TestCollectingPublisherSubscriber utilities. Review TestRecordStore and TestDnsServer, including DelegateRecordStore and store; done means expected results are verified with the general utilities, record sequencing is controllable, and unused test-server machinery is addressed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- networking, testing
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100