Open Bug 472007 Opened 11 years ago Updated 5 years ago
force tests to not rely on external resources
Tests shouldn't rely on external resources that might change, disappear, or causing timing oddities due to network glitches. I think we've hit this a couple of times in the past, when tests were loading images from a real server (w3.org or something). AFAIK those tests were fixed. Asac just hit an interesting variation, where test_bug407303.js was failing because it tries to load "jar:http://test.invalid/test.jar!/index.html", and his ISP's dodgy DNS was actually resolving test.invalid (!). Seems like we should just force the tests to proxy everything though https.js, and have httpd.js log/explode when an unknown resource is requested. Mochitests currently use a FindProxyForURL() that seems to only proxy certain test hosts. I'm hoping we don't actually have tests that need to load non-proxied content... Maybe those could handled as special exceptions?
If you send a request with an unrecognized Host header (or host in the absolute-URI part of the Request-Line, but I don't think we do that except in the proxying case), the response MUST have the 400 status code; that seems like it would be adequate to me. If we pull out the code to require a recognized host in FindProxyForURL, that would fix things quite handily (except for non-http(s) protocols, but one step at a time :-) ). This will break my use case of firing up Mochitest with an invalid --test-path and then loading some arbitrary page to check for its causing a leak, but maybe a --allow-external-sites option is the best way to address that. This nit alone certainly isn't enough to make this bug WONTFIX, of course.
Component: httpd.js → Mochitest
QA Contact: httpd.js → mochitest
This might have helped us track down bug 486489 faster, where the urlclassifier service was running in the background updating the database during mochitests, causing random test failures/leaks. If we had prevented the requests from happening and warned about it, it would have been a lot more obvious.
For reference, here is a patch I started working on for this that seemed to work except a bunch of tests were failing in ways I couldn't understand.
You need to log in before you can comment on or make changes to this bug.