Closed
Bug 1257249
Opened 9 years ago
Closed 8 years ago
"TypeError: must be string or pinned buffer, not bytearray" log spam from various referrer-policy tests
Categories
(Testing :: web-platform-tests, defect)
Tracking
(firefox52 fixed, firefox53 fixed)
RESOLVED
FIXED
mozilla53
People
(Reporter: RyanVM, Assigned: jgraham)
References
Details
This looks like an issue in the test, so filing it under Testing::w-p-t for now.
https://treeherder.mozilla.org/logviewer.html#?job_id=155465&repo=ash
Comment 1•9 years ago
|
||
Can't repro locally; might depend on exact python/cStringIO versions.
Reporter | ||
Comment 2•9 years ago
|
||
This affects Linux and OSX in automation and is really spammy. AFAICT, Windows is unaffected. What can we do to work around it in our CI environments?
Updated•9 years ago
|
Flags: needinfo?(james)
Comment 3•9 years ago
|
||
Indeed, this appears to be a Python version problem as suggested in comment 1. The error (which induces test failures involving image creation, complicating bug 1265864) was fixed in Python 2.7.4 by https://bugs.python.org/issue10212.
The config at https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/configs/web_platform_tests/prod_config.py#21 is explicitly setting up the virtualenv to use "/tools/buildbot/bin/python" when I suspect it really wants to be using "/tools/python27/bin/python2.7" provided by the "mozilla-python27-2.7.7-1.el6" package.
Gonna do a naive try push momentarily that should automatically spam here.
Blocks: 1265864
Comment 4•9 years ago
|
||
Comment 5•9 years ago
|
||
That didn't work, so I did another push to find out what the python version actually is. Unsurprisingly, it's 2.7.3. Somewhat surprisingly, when I looked further, I see something mentioning mock packages and "mozilla-python27", suggesting there is logic intentionally defeating Python 2.7.7 from being installed which explains why the line: "Package mozilla-python27-2.7.7-1.el6.x86_64 already installed and latest version" is so misleading.
Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3772a8456985&selectedJob=19743755
Logs with structured logging spamminess removed:
PYTHON VERSION:
2.7.3 (default, Apr 20 2012, 22:39:59)
[GCC 4.6.3]
sys.version_info(major=2, minor=7, micro=3, releaselevel='final', serial=0)
And it does confirm the binary is:
/builds/slave/test/build/venv/bin/python
I'm gonna flail around and try and ping people on IRC now to help, but anyone reading this bug is more than welcome to jump in or ping me as "asuth" on IRC. Thanks!
Assignee | ||
Comment 6•9 years ago
|
||
jlund: Can we upgrade the python version on the test slaves?
Flags: needinfo?(james) → needinfo?(jlund)
Comment 7•9 years ago
|
||
(In reply to James Graham [:jgraham] from comment #6)
> jlund: Can we upgrade the python version on the test slaves?
sounds like you just need a minor bump? 2.7.4? So maybe we can get away with upgrading just test machines if 2.7.x is still used everywhere..
tracking: Bug 1266240
Flags: needinfo?(jlund)
Reporter | ||
Comment 8•9 years ago
|
||
We know it's affecting bug 1257249 already. Anecdotally, I feel like old python versions have bit others in the past, but I don't have any concrete examples offhand.
Comment 9•9 years ago
|
||
(In reply to Jordan Lund (:jlund) bug 1266240 from comment #5)
> ftr - I'm not actively working on this. please yell if this is a blocker and
> someone from releng needs to pick it up.
(Replying here too to bug 1266240 comment 5 since that bug pre-supposes the solution.)
Bug 1265864 (blocked by bug 1257249 which Ryan mentions) is a real bummer because when developers run these web-platform-tests locally they will get different results from what the test/try servers will get. The workflow for the tests is that you can then update the manifests (automatically based on the results). Developers who have fixed things may think they fixed those tests or the manifests just need to be updated, update the .ini expectations, push to try, get failures, have to re-read the bug, etc. And since Python 2.7.4 was released in April, 2013 (even debian stable is on 2.7.9-1! https://packages.debian.org/stable/python/python), this version mismatch is likely to happen for all developers.
If there are more fundamental fixes like updating the build machines to less outdated system images or moving these tests to be run on taskcluster with docker so modern python can be used and these are actively being worked, then it could make sense to wait for those. I may be able to help with the taskcluster option if that's where things are headed and someone can provide pointers and it doesn't require touching releng things I don't have access to. (Noting that my taskcluster/docker experience is very limited; I've basically used and looked at working things and said "that makes sense".)
Comment hidden (Intermittent Failures Robot) |
Comment 12•8 years ago
|
||
The proper fix for WPT is to use io.BytesIO or io.StringIO instead of cStringIO.StringIO (or StringIO.StringIO). The io.{BytesIO,StringIO} classes enforce that types written and read are consistent. e.g. a BytesIO will not allow writing a unicode instance (only str) (using Python 2 types). StringIO.StringIO, however, allows both str and unicode to be written and does coercion as appropriate.
Another valid fix is to use StringIO.StringIO instead of cStringIO, as the bug is only present in cStringIO.
Keep in mind that if performance is important, cStringIO is your best bet. io.BytesIO and io.StringIO are notoriously slow on Python 2 :( But for the use in image.py, I think BytesIO or the non-C version of StringIO would be suitable.
Ideally we'd get Python 2.7.12 on the testers. I'm sick of working around Python bugs that have been fixed in modern versions.
Comment 13•8 years ago
|
||
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f863d5050b3f
Replace a use of cStringIO with StringIO to avoid a problem with old Python versions, r=Ms2ger
Comment 14•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Reporter | ||
Updated•8 years ago
|
Whiteboard: [checkin-needed-aurora]
One file this patch touches (testing/web-platform/meta/mathml/presentation-markup/fractions/frac-1.html.ini) doesn't exist on aurora, and four other files have conflicts when this patch is applied:
testing/web-platform/meta/FileAPI/blob/Blob-constructor.html.ini
testing/web-platform/meta/fetch/api/basic/request-headers-worker.html.ini
testing/web-platform/meta/fetch/api/basic/request-headers.html.ini
testing/web-platform/meta/presentation-api/controlling-ua/startNewPresentation_error.html.ini
I wouldn't object to a rebased patch to work around these conflicts.
Comment 16•8 years ago
|
||
(In reply to Wes Kocher (:KWierso) from comment #15)
> One file this patch touches
> (testing/web-platform/meta/mathml/presentation-markup/fractions/frac-1.html.
> ini) doesn't exist on aurora, and four other files have conflicts when this
> patch is applied:
> testing/web-platform/meta/FileAPI/blob/Blob-constructor.html.ini
>
> testing/web-platform/meta/fetch/api/basic/request-headers-worker.html.ini
>
> testing/web-platform/meta/fetch/api/basic/request-headers.html.ini
>
> testing/web-platform/meta/presentation-api/controlling-ua/
> startNewPresentation_error.html.ini
>
> I wouldn't object to a rebased patch to work around these conflicts.
James do you know who could you take a look ?
Flags: needinfo?(james)
Whiteboard: [checkin-needed-aurora]
Assignee | ||
Comment 17•8 years ago
|
||
FWIW I think it should work fine to just use the version on aurora for all of those files (the changes here are just from reserializing the expected data and don't have any semantic changes). Do you need me to prepare such a patch or are you able to handle it from here?
Flags: needinfo?(james)
Reporter | ||
Comment 18•8 years ago
|
||
I'll take a look at this when I get a chance.
Reporter | ||
Comment 19•8 years ago
|
||
Yeah, the image.py change applies cleanly to Aurora, so I'll try landing just that.
Flags: needinfo?(ryanvm)
Reporter | ||
Comment 20•8 years ago
|
||
bugherder uplift |
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•