Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

werkzeug 0.15.0: mystery "generated" module when invoking coverage #1487

Closed
asottile opened this issue Mar 22, 2019 · 15 comments
Closed

werkzeug 0.15.0: mystery "generated" module when invoking coverage #1487

asottile opened this issue Mar 22, 2019 · 15 comments
Milestone

Comments

@asottile
Copy link
Contributor

This seems to be a minimal reproduction:

(note I made a virtualenv one level up to avoid that being in the coverage output)

.coveragerc

[run]
source = .

t.py

import flask

app = flask.Flask(__name__)

@app.route('/foo/<a>')
def hello(a):
    return 'world'


# pretend this is a test
with app.test_request_context():
    with app.test_client() as client:
        client.get(flask.url_for('hello', a='1'))

runtime

$ bash -xc 'coverage erase && coverage run t.py && coverage report && coverage xml'
+ coverage erase
+ coverage run t.py
+ coverage report
generated   NoSource: No source for code: '/tmp/t/generated'.
Aborting report output, consider using -i.
Name    Stmts   Miss  Cover
---------------------------
t.py        7      0   100%
+ coverage xml
No source for code: '/tmp/t/generated'.
Aborting report output, consider using -i.

triage (?)

This appears to come from here:

"generated",

or, at least, that's the only instance of 'generated' or "generated" in my site-packages -- so I may be guessing a bit here.

This is causing CI failures in https://github.com/asottile/git-code-debt

workaround

A workaround appears to be setting this:

[run]
# ...
omit =
    *generated

in .coveragerc

though ideally I wouldn't have to exclude this 🤔

asottile added a commit to asottile/git-code-debt that referenced this issue Mar 22, 2019
@davidism
Copy link
Member

davidism commented Mar 22, 2019

When Rule generates its build code, CodeType.filename is set to "generated". It should probably be set to __file__.

code_args = [
argcount,
len(self.var),
peak_stack + len(self.var),
flags,
ops,
tuple(self.consts),
(),
tuple(self.var),
"generated",
"<builder:%r>" % self.rule.rule,
1,
b"",
]

@davidism davidism added this to the 0.15.2 milestone Mar 22, 2019
@asottile
Copy link
Contributor Author

I think None would also work (?)

@davidism
Copy link
Member

@asottile I'll give that a try. Was about to write this whole thing, so I'll post anyway. :-)


For clarity, the code above is better as a dict:

code_args = {
    "argcount": argcount,
    "nlocals": len(self.var),
    "stacksize": peak_stack + len(self.var),
    "flags": flags,
    "codestring": ops,
    "constants": tuple(self.consts),
    "names": (),
    "varnames": tuple(self.var),
    "filename": __file__,
    "name": "_build_unknown" if append_unknown else "_build",
    "firstlineno": 1,
    "lnotab": b"",
}
if not PY2:
    code_args["kwonlyargcount"] = 0
co = types.CodeType(**code_args)

I'm not really sure what the correct answer is. It's weird that this isn't causing the same error in Werkzeug's own coverage. How will coverage react if I tell it that the generated function is part of that module, what line do I say it starts on, what does it do when the lnotab table is empty, etc.?

@davidism
Copy link
Member

CodeType doesn't work with keyword arguments. :-(

@asottile
Copy link
Contributor Author

inspecting the coverage data I see two arcs, one from (-1, 1) and the other from (1, -1), nonsense it seems

I think werkzeug itself avoids this due to:

werkzeug/setup.cfg

Lines 18 to 22 in 86f7bdf

[coverage:run]
branch = True
source =
werkzeug
tests

Which avoids the "generated" file at .

@davidism
Copy link
Member

Tests still pass with None, could you try patching that locally and see if it fixes your project's coverage?

@asottile
Copy link
Contributor Author

woops, my suggestion was wrong, and I'm concerned about the test suite now 😨

+ coverage run t.py
Traceback (most recent call last):
  File "t.py", line 3, in <module>
    app = flask.Flask(__name__)
  File "/tmp/venv/lib/python3.6/site-packages/flask/app.py", line 562, in __init__
    view_func=self.send_static_file
  File "/tmp/venv/lib/python3.6/site-packages/flask/app.py", line 66, in wrapper_func
    return f(self, *args, **kwargs)
  File "/tmp/venv/lib/python3.6/site-packages/flask/app.py", line 1216, in add_url_rule
    self.url_map.add(rule)
  File "/tmp/venv/lib/python3.6/site-packages/werkzeug/routing.py", line 1563, in add
    rule.bind(self)
  File "/tmp/venv/lib/python3.6/site-packages/werkzeug/routing.py", line 710, in bind
    self.compile()
  File "/tmp/venv/lib/python3.6/site-packages/werkzeug/routing.py", line 766, in compile
    self._build = self._compile_builder(False)
  File "/tmp/venv/lib/python3.6/site-packages/werkzeug/routing.py", line 1129, in _compile_builder
    return self.BuilderCompiler(self).compile(append_unknown)
  File "/tmp/venv/lib/python3.6/site-packages/werkzeug/routing.py", line 1120, in compile
    co = types.CodeType(*code_args)
TypeError: code() argument 10 must be str, not None

Setting it to "" instead seems to work

@davidism
Copy link
Member

🤦‍♂️ I didn't actually set it to None.

@asottile
Copy link
Contributor Author

It looks like exec uses "<string>" for that value:

>>> s = 'print("hi")\ndef f(): pass\nprint(repr(f.__code__.co_filename))'
>>> exec(s)
hi
'<string>'

@davidism
Copy link
Member

Yeah, I was looking at compile too. Does coverage skip those?

@asottile
Copy link
Contributor Author

yep!

(this is in coverage/control.py in the released version I'm using, but here's the moved source):

https://github.com/nedbat/coveragepy/blob/28d223b4265242e08dac4bb016270dbc1b4482cb/coverage/inorout.py#L240-L245

@davidism
Copy link
Member

davidism commented Mar 22, 2019

Yeah, just found that, it skips empty or < prefix. Debating whether I should use something specific like "<werkzeug routing>" or just the empty string.

@asottile
Copy link
Contributor Author

I'd go with "<werkzeug routing>"

@edk0
Copy link
Contributor

edk0 commented Mar 22, 2019

(I got a new job, I'm trying to find the time to get back into this...) <werkzeug routing> seems right to me. I did that for the function name, so not doing it for the file seems like a bit of an oversight anyway.

@davidism
Copy link
Member

No worries, I've already got a branch I'll PR later.

asottile added a commit to asottile/git-code-debt that referenced this issue Apr 23, 2019
asottile added a commit to asottile/git-code-debt that referenced this issue Apr 23, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants