[talos] Replace mozhttpd with wptserve
Categories
(Testing :: Talos, enhancement, P1)
Tracking
(firefox72 fixed)
Tracking | Status | |
---|---|---|
firefox72 | --- | fixed |
People
(Reporter: raphael, Assigned: b4hand)
References
Details
Attachments
(1 file)
According to bug 1150512 the webserver used in Talos "mozhttpd" is considered unsupported. davehunt suggested to replace mozhttpd with wptserve for Python projects in mozilla-central.
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
Updated•5 years ago
|
Comment 2•5 years ago
|
||
I believe this is :b4hand's try push:
Comment 3•5 years ago
|
||
Please see comments in phab, I'm seeing a strange PNG file pop-up during tp5o with the patch applied.
Also there are some failures in the try push (Comment 2), unsure if they are related to the server change or not. Here's a try push of talos tests on OSX only with latest central (with patch NOT applied) to see if the failures are pre-existing:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=86237e947d4e0327e670c7ecfcb99d5b2193d2f4
Assignee | ||
Comment 4•5 years ago
|
||
It seems the pageset that is being loaded tries to load some images within an iframe. This was not a problem when using mozhttpd because it seems to wrap everything with a header.
When switching to the wptserve server, we no longer do this and now Firefox is being prompted to download the images since their headers are now application/octet-stream
.
I followed the steps here and was able to get the page to load without prompting for a download.
Comment 5•5 years ago
|
||
Thanks Benjamin, much appreciated!
So if I'm understanding correctly, there are two options here:
- Update the very old tp5n.zip pageset archive (on tooltool) and add a '.headers' file for the offending page (I believe it is 'bild.de').
- or -
- We remove the particular offending page ('bild.de') from the list of URLs used by tp5o [0]. I think that may let us run the existing tests and solve the problem, without updating the pageset archive (but need to test that out).
:mconley, davehunt, do you have any preference here as to which way we should go? The tp5n.zip pageset is quite old, will it really matter if we remove one test page?
Comment 6•5 years ago
|
||
The source linked in comment 4 returns a content type of JSON, is this really what we're doing for the PNG in the existing tests? My preference would be to match the headers that we're currently returning, if this is as simple as adding a .headers
file to the tp5 pageset. There's a bigger discussion on replacing the tp5 pageset, but I don't think that should block progress here.
Out of interest, have we tested the performance impact of switching mozhttpd to wptserve? If there's an impact expected then we should notify the performance sheriffs as they can expect a new baseline.
Comment 7•5 years ago
|
||
I agree that the tp5n set is old. In fact, it's probably so old that we might be losing coverage due to newer web features and web APIs since the original recording was made. I'd honestly prefer a new set of recordings - that'd be useful for making the tests more reflective for Fission too.
Comment 8•5 years ago
|
||
Thanks guys. Updating the tp5 pageset is quite a big task, as tp5 uses raw html source files on disk, not mitmproxy playback recordings. My understanding is updating the pageset would require modifying all source pages to remove any dynamic external links to ensure all dependencies are in the local files. Can be done of course but would be a pretty big time consuming project. Another option is to migrate talos tp5 to use mozproxy/mitmproxy and use the tp6 recordings instead. However that would also be a pretty big change but do-able of course.
Anyway I think those questions are outside of this immediate issue (replacing mozhttpd with wptserve). Looks like the preference here is to update the tp5 pageset to fix the issue that is introduced by the switch to wptserve.
:b4hand, do you want to add that to your patch (adding the .headers file)? If you get an updated page set working locally and provide a new tp5n.zip file, I can upload it to tooltool.
Assignee | ||
Comment 9•5 years ago
|
||
I think it makes more sense to change the index.html within the pageset to fix the problem, vs adding a header file. Changing the iframes to img tags works. I can provide that pageset zip too.
Comment 10•5 years ago
|
||
(In reply to Benjamin Forehand Jr[:b4hand] from comment #9)
I think it makes more sense to change the index.html within the pageset to fix the problem, vs adding a header file. Changing the iframes to img tags works. I can provide that pageset zip too.
Great thanks Benjamin!
Assignee | ||
Comment 11•5 years ago
|
||
I provided you with a pageset, could you confirm that it is uploaded to tooltool? I can run a job on Try after that. Thanks!
Comment 13•5 years ago
|
||
(In reply to Benjamin Forehand Jr[:b4hand] from comment #12)
Please see comment 11.
Thanks, uploaded the provided 'tp5n.zip' to tooltool and sent you the new manifest.
Comment 14•5 years ago
|
||
Hi Benjamin, I'm just awaiting a green try run please with your latest updates, thanks!
Assignee | ||
Comment 15•5 years ago
|
||
I am still having some issues with Try. All my jobs (4) never started. I am reaching out to the firefox-ci team today to hopefully get some help. Sorry about the delay.
Comment 16•5 years ago
|
||
(In reply to Benjamin Forehand Jr[:b4hand] from comment #15)
I am still having some issues with Try. All my jobs (4) never started. I am reaching out to the firefox-ci team today to hopefully get some help. Sorry about the delay.
Thanks for the update Benjamin, if you want me to push your patch to try on your behalf later today just let me know.
Assignee | ||
Comment 17•5 years ago
|
||
I think that would work. I am stuck in a rebase/patch/someone_help land with mercurial, and if the patch just needs to be tested, please push on try for me. Thanks.
Comment 18•5 years ago
|
||
Comment 19•5 years ago
|
||
Pushed by rwood@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5dd01231a71c [talos] Replace mozhttpd with wptserve; r=rwood,perftest-reviewers
Comment 20•5 years ago
|
||
bugherder |
Description
•