Closed Bug 1257249 Opened 8 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)

Product:
Testing
Component:
web-platform-tests
Version:
Version 3
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox52 fixed, firefox53 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox52 --- fixed
firefox53 --- fixed

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
Can't repro locally; might depend on exact python/cStringIO versions.
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?
Flags: needinfo?(james)
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
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!
jlund: Can we upgrade the python version on the test slaves?
Flags: needinfo?(james) → needinfo?(jlund)
Depends on: 1266240
(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)
See Also: → 711299
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.
(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".)
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.
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
https://hg.mozilla.org/mozilla-central/rev/f863d5050b3f
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
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.
(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]
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)
I'll take a look at this when I get a chance.
Assignee: nobody → james
status-firefox52: --- → affected
Flags: needinfo?(ryanvm)
Yeah, the image.py change applies cleanly to Aurora, so I'll try landing just that.
Flags: needinfo?(ryanvm)
You need to log in before you can comment on or make changes to this bug.