1474531 - In lcov_rewriter.py iteritems attribute is not supported by Python 3

Status: RESOLVED FIXED

(Testing :: Code Coverage, defect)

Description

[assiya.khuzyakhmetova]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20180310025718

Steps to reproduce:

Used lcov_rewriter.py in Python 3 project to rewrite JS coverage .info files


Actual results:

Files are not rewritten because of the following error: AttributeError: 'dict' object has no attribute 'iteritems'


Expected results:

Files should be rewritten into a single .info file.

Comment 1

[assiya.khuzyakhmetova]

Attachment: Bug 1474531 - Replace iteritems with viewitems in lcov_rewriter.py.

Review commit: https://reviewboard.mozilla.org/r/256010/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/256010/

Comment 2

[Marco Castelluccio [:marco]]

Comment on attachment 8990997 [details]
Bug 1474531 - Replace iteritems with viewitems in lcov_rewriter.py.

https://reviewboard.mozilla.org/r/256010/#review262854

::: Pipfile:11
(Diff revision 1)
>  [dev-packages]
>  
>  [packages]
>  attrs = "==18.1.0"
>  blessings = "==1.7"
> +future = "==0.16.0"

I'm not sure it's fine to change the top-level Pipfile just for this.
We could implement something similar to future.utils.viewitems ourselves in lcov_rewriter.py:
```
def iter_items(d, **kwargs):
    func = getattr(d, "viewitems", None)
    if not func:
        func = d.items
    return func(**kwargs)
```

Let's ask :ahal what he thinks. If we are planning to use future for other things too then I guess it's fine.

Comment 3

[Marco Castelluccio [:marco]]

See comment 2.

Comment 4

[assiya.khuzyakhmetova]

Comment on attachment 8990997 [details]
Bug 1474531 - Replace iteritems with viewitems in lcov_rewriter.py.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/256010/diff/1-2/

Comment 5

[Marco Castelluccio [:marco]]

The patch was updated to use six instead.

Comment 6

[Marco Castelluccio [:marco]]

Comment on attachment 8990997 [details]
Bug 1474531 - Replace iteritems with viewitems in lcov_rewriter.py.

https://reviewboard.mozilla.org/r/256010/#review262898

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3ffd10f22f12
Replace iteritems with viewitems in lcov_rewriter.py. r=marco

Comment 8

[Andrew Halberstadt [:ahal]]

Yeah if there's a good reason to depend on futures, we can do it. But I'd prefer to just stick to six if we can help it.

Another option for this particular case would have been to s/iteritems/items. Unless you're iterating over a massive amount of data, then it's probably not a huge deal to just use 'items()' with python 2 as well.

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/3ffd10f22f12