Autocomplete tester #28
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
13 tasks
The callback function doesn't have to be use for an output generator as it was previously named.
This helps reinforce that they are related
exec_test_case.js has a lot of responsiblities now handled by smaller more focused modules: validate_test_suites.js: perform validation on test suites. Some errors are recoverable, some are not gather_test_suites.js: find all test suites, filter out tests that shouldn't be run based on test type command line parameter gather_test_urls.js: collect all the URLs from all the test cases that should be fetched request_urls.js: perform all the fetching of URLs eval_tests.js: run all the scoring code on each of our tests based on the responses previously fetched analyze_results.js: perform any final analysis required on the tests results. currently it simply tallys up the pass/fail/regression/improvement count test_suite_helpers.js: miscellaneous helper code used in a few places
The return code is used as the return code for the entire process. Previously each output generator was responsible for calling process.exit which was easy to forget about.
The functionality that used to be in these files is now separated more cleanly into many other files.
The `request_urls` module uses the [HTTP.Agent](https://nodejs.org/api/http.html#http_class_http_agent) class to limit the maximum number of open HTTP sockets (by default with no agent configured, no artificial limit on the number of sockets is set). This means that instead of attempting to space out the actual calling of request(), all the request() calls can be made at once, and then they will only actually be performed a few at a time. In my testing this is able to completely eliminate the possibility of overloading our server with too many requests at once, while allowing for much faster test suite execution. As an added (small) bonus, HTTP keepalives are now set so the overhead of initializing a connection for each request is removed, making things even faster. Finally, when results are cached in Fastly, the test suite can execute extremely fast: I've seen over 400 request/second handled just fine. Oh, and it's a lot less code :)
These are somewhat confusing because, for example, the autocomplete tests can show that a completely correct result was first, but the test still failed (because there was another expected result that wasn't found).
This is much more friendly to dev.
There seems to an [issue](nodejs/node#2148) with some versions of Node that cause one of stderr and stdout to be buffered, while the other is not. This jumbles output if both are used.
It now has to be able to print the circular test case structure, and URL info has slightly changed.
The test evaluation code can handle 500 responses, we just need to store them.
These basically mean autocomplete was not helpful over search.
Production and dev have different timeouts
This is slower when testing against fastly, but multiple sockets can easily overwhelm prodbuild or dev.
Otherwise, when some requests are currently being retried, the script can exit early before printing results (but usually after waiting through almost all of the fetching).
This reverts commit a21eb16. Because of the timeout functionality of Elasticsearch, even using only one socket can overwhelm a server when sending off requests as fast as possible, if timeouts start coming back, requests need to be spaced out and ExponentialBackoff is really good at that.
0 is now acceptable for minimum delay since all requests are queued through a single socket. However, in the case of retries it's desireable to slow down more severely to allow the Elasticsearch cluster to finish processing any timed-out requests, so the exponent is increased. Finally the maximum timeout is doubled to 20 seconds since an overloaded Elasticsearch server tends to thrash (our 4 core prodbuild server will sometimes have a load of 20+)
orangejulius
force-pushed
the
autocomplete-tester
branch
from
d0c5468 to
79ff088
Compare
setTimeout and setInterval can't handle a 0ms delay, and ExponentialBackoff defaults to 50ms when passed 0ms
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is it! A way too big pull request that includes a lot of refactoring (also in #25, which is now closed), and adds the ability to test autocomplete.
It includes not just a new output format for autocomplete, but some additional interesting autocomplete related statistics.
Here's the latest screenshot of the acceptance test output
Fixes #23