Content-Transfer-Encoding: binary X-RT-Interface: Web Content-Disposition: inline In-Reply-To: X-RT-Original-Encoding: utf-8 Message-ID: X-Mailer: MIME-tools 5.508 (Entity 5.508) References: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" RT-Send-CC: Content-Length: 2825 Reviewed the tests and documentation for commit addc4c6e67f0a4a05bada67d4a6d917657fecc9c Documentation --- The documentation for stale-answer-ttl states that for stale answers to be returned, max-stale-ttl must be set to a non-zero value and they must not have been disabled by rndc". However, from the named.conf files in the test it appears that stale-answer-enable also needs to be set. In addition, the documentation does not mention stale-answer-enable - the option appears to be called serve-stale-enable there. Tests (general comments) --- I note that the records returned by ans.pl have a TTL of 1 second, and the script waits for 1 second for them to become stale. To avoid any possible timing problems when running the tests, I suggest that the sleep intervals be lengthened to 2 seconds. It would be helpful if the messages in the test script were more informative. For example, "check rndc serve-stale status" messages could be expanded to include whether it was expected that the status is enabled or disabled. When the test of invalid rndc command is made, preceding the messages with a message stating that invalid commands are being tested would have been helpful. I was perplexed at first by the messages "check rndc serve-stale" and "check rndc serve-stale unknown", thinking they were valid commands. When checking the test code, it took a little time to realise that resolver ns3 was being tested and not ns1. (I was puzzled by the fact that the last loaded configuration for ns1 contained a max-stale-ttl of 7200, yet a value of 604800 was being returned by rndc.) Was there a reason for this - why not just use a new configuration and reload (as done earlier in the script)? Tests (specific) --- A summary of the test coverage against requirements is given in the cross reference of the tests against the requirements in the internal wiki. The main issues are (numbers are the requirement number): 1.1 No check that if the feature is explicitly disabled in the configuration file (as opposed to not being configured), it is disabled in practice. 2.1 There is no explicit test that an upstream query is being made before a stale record is served. (This could be tested by extending "ans.pl" to return a count of queries made and testing that before and after requesting stale records.) 2.4 All records in the test have a TTL of 1. There is no check that the TTL of the stale record returned is set (capped?) by the stale-answer-ttl value. 2.6 There is no check that the max-stale-ttl value caps the amount of time that a stale record can be served. 2.8 No test that a stale positive value is updated by a NODATA or NXDOMAIN. There are no tests for the requirements in section 3 (interaction with other features), but these are tests that can be implemented between the alpha and beta.