Introduce --report-log by nicoddemus · Pull Request #5980 · pytest-dev/pytest

nicoddemus · 2019-10-16T22:35:00Z

EDIT:

This should now be ready for final review.

OLD:

Started working on this, before progressing further I want to gather opinions if the direction is right.

Given this simple test file:

def test_ok():
    pass

def fail():
    assert 0

def test_failure():
    fail()

Here is the file produced:

{"nodeid": "", "outcome": "passed", "longrepr": null, "result": null, "sections": [], "_report_type": "CollectReport"}
{"nodeid": "test_foo.py", "outcome": "passed", "longrepr": null, "result": null, "sections": [], "_report_type": "CollectReport"}
{"nodeid": "test_foo.py::test_ok", "location": ["test_foo.py", 0, "test_ok"], "keywords": {"test_ok": 1, "test_foo.py": 1, ".tmp": 1}, "outcome": "passed", "longrepr": null, "when": "setup", "user_properties": [], "sections": [], "duration": 0.000990152359008789, "_report_type": "TestReport"}
{"nodeid": "test_foo.py::test_ok", "location": ["test_foo.py", 0, "test_ok"], "keywords": {"test_ok": 1, "test_foo.py": 1, ".tmp": 1}, "outcome": "passed", "longrepr": null, "when": "call", "user_properties": [], "sections": [], "duration": 0.0, "_report_type": "TestReport"}
{"nodeid": "test_foo.py::test_ok", "location": ["test_foo.py", 0, "test_ok"], "keywords": {"test_ok": 1, "test_foo.py": 1, ".tmp": 1}, "outcome": "passed", "longrepr": null, "when": "teardown", "user_properties": [], "sections": [], "duration": 0.0010075569152832031, "_report_type": "TestReport"}
{"nodeid": "test_foo.py::test_failure", "location": ["test_foo.py", 8, "test_failure"], "keywords": {"test_foo.py": 1, "test_failure": 1, ".tmp": 1}, "outcome": "passed", "longrepr": null, "when": "setup", "user_properties": [], "sections": [], "duration": 0.0, "_report_type": "TestReport"}
{"nodeid": "test_foo.py::test_failure", "location": ["test_foo.py", 8, "test_failure"], "keywords": {"test_foo.py": 1, "test_failure": 1, ".tmp": 1}, "outcome": "failed", "longrepr": {"reprcrash": {"path": "D:\\projects\\pytest\\.tmp\\test_foo.py", "lineno": 6, "message": "assert 0"}, "reprtraceback": {"reprentries": [{"type": "ReprEntry", "data": {"lines": ["    def test_failure():", ">       fail()"], "reprfuncargs": {"args": []}, "reprlocals": null, "reprfileloc": {"path": "test_foo.py", "lineno": 10, "message": ""}, "style": "long"}}, {"type": "ReprEntry", "data": {"lines": ["    def fail():", ">       assert 0", "E       assert 0"], "reprfuncargs": {"args": []}, "reprlocals": null, "reprfileloc": {"path": "test_foo.py", "lineno": 6, "message": "AssertionError"}, "style": "long"}}], "extraline": null, "style": "long"}, "sections": [], "chain": [[{"reprentries": [{"type": "ReprEntry", "data": {"lines": ["    def test_failure():", ">       fail()"], "reprfuncargs": {"args": []}, "reprlocals": null, "reprfileloc": {"path": "test_foo.py", "lineno": 10, "message": ""}, "style": "long"}}, {"type": "ReprEntry", "data": {"lines": ["    def fail():", ">       assert 0", "E       assert 0"], "reprfuncargs": {"args": []}, "reprlocals": null, "reprfileloc": {"path": "test_foo.py", "lineno": 6, "message": "AssertionError"}, "style": "long"}}], "extraline": null, "style": "long"}, {"path": "D:\\projects\\pytest\\.tmp\\test_foo.py", "lineno": 6, "message": "assert 0"}, null]]}, "when": "call", "user_properties": [], "sections": [], "duration": 0.0009996891021728516, "_report_type": "TestReport"}
{"nodeid": "test_foo.py::test_failure", "location": ["test_foo.py", 8, "test_failure"], "keywords": {"test_foo.py": 1, "test_failure": 1, ".tmp": 1}, "outcome": "passed", "longrepr": null, "when": "teardown", "user_properties": [], "sections": [], "duration": 0.0, "_report_type": "TestReport"}

Some things I believe can be improved:

"_report_type" is currently private, I think it should be promoted to public as "report_type".
We could add more events:
- session start/finish
- collection start/finish
- warnings
It might make sense to accept multiple log paths, so tools could consume the log while allowing users to use --report-log themselves. For example, an IDE might want to use --report-log to update its UI, passing a file it owns to --report-log, without affecting users that might also be using --report-log.

RonnyPfannschmidt

Overall good start, and believe it is already better than resultlog

src/_pytest/report_log.py

nicoddemus · 2019-10-17T13:07:29Z

Overall good start, and believe it is already better than resultlog

Cool. Any comments about points 1, 2, and 3 that I posted?

nicoddemus · 2019-10-17T13:09:03Z

Another point that occurred to me:

We might add a "config" entry at the start or together with "session start", containing rootdir, ini, and loaded plugins (similar to what we see on the banner at the start of the test session).

RonnyPfannschmidt · 2019-10-17T15:03:21Z

wrt reporttype public/nonpublic - we we should use $reporttype or $type as a marker that cant be a python value - this needs a breaking release as far as i understood

wrt creating folders - i believe we shouldn't create them, as that may hide user errors (imagine a typo in the path and the reports end in nirvana instead of a report folder

wrt multiple log paths, i'd rather see a report stream hook that for the report log plug-in serializes, and ides would either use serialize as well, or directly use the richer objects

wrt more events, happily as follow-up and as pytest serialize/de-serialize supports them

we should have a header telling the pytest version and supported serialization types

nicoddemus · 2019-10-17T15:25:40Z

Hi @RonnyPfannschmidt, thanks for the feedback!

wrt reporttype public/nonpublic - we we should use $reporttype or $type as a marker that cant be a python value - this needs a breaking release as far as i understood

Hmm why "$report_type" instead of just "report_type"? Just to mark it as "special"?

wrt creating folders - i believe we shouldn't create them, as that may hide user errors (imagine a typo in the path and the reports end in nirvana instead of a report folder

I disagree; the same reasoning could be made by the user mistyping the filename (--report-log=myreport.json instead of --report-log=my_report.json). Another point is consistency with junitxml.

If the report ends up in a different place, the user will notice and correct it, facts of life. 😁

wrt multiple log paths, i'd rather see a report stream hook that for the report log plug-in serializes, and ides would either use serialize as well, or directly use the richer objects

I see, but I was thinking more along the lines of system where they couldn't write hooks (say implemented in a language other than Python). Otherwise, they could just implement hooks directly and capture the events there.

wrt more events, happily as follow-up and as pytest serialize/de-serialize supports them

Sure. 👍

we should have a header telling the pytest version and supported serialization types

Something like:

{"pytest-version": "5.3.0", "serialized-types": ["Header", "CollectReport", "TestReport"], "report_type": "Header"}

?

RonnyPfannschmidt · 2019-10-17T16:28:03Z

Hi @RonnyPfannschmidt, thanks for the feedback!

wrt reporttype public/nonpublic - we we should use $reporttype or $type as a marker that cant be a python value - this needs a breaking release as far as i understood

Hmm why "$report_type" instead of just "report_type"? Just to mark it as "special"?

the type is not part of the serialized objects normal namespace, the $ simply prevents of ever having to account for a name conflict on that level

wrt creating folders - i believe we shouldn't create them, as that may hide user errors (imagine a typo in the path and the reports end in nirvana instead of a report folder

I disagree; the same reasoning could be made by the user mistyping the filename (--report-log=myreport.json instead of --report-log=my_report.json). Another point is consistency with junitxml.

i still disagree, but i can make do with this setup

If the report ends up in a different place, the user will notice and correct it, facts of life. grin

wrt multiple log paths, i'd rather see a report stream hook that for the report log plug-in serializes, and ides would either use serialize as well, or directly use the richer objects

I see, but I was thinking more along the lines of system where they couldn't write hooks (say implemented in a language other than Python). Otherwise, they could just implement hooks directly and capture the events there.

making a small local plug-in the environment invokes is trivial and done by many systems not implemented in python already,

what would be interesting is having the log report as stdout stream instead of the normal io

then pytest could be invoked by the system and just report details directly
(this would need the "terminalwriter" to hand out ux commands instead of doing them)

wrt more events, happily as follow-up and as pytest serialize/de-serialize supports them

Sure. +1

we should have a header telling the pytest version and supported serialization types

Something like:
{"pytest-version": "5.3.0", "serialized-types": ["Header", "CollectReport", "TestReport"], "report_type": "Header"}

exactly

?

RonnyPfannschmidt · 2019-10-17T16:37:44Z

btw, i can imagine something like pytest-dbus, where pytest would report events to dbus -> or an api

nicoddemus · 2019-10-26T15:42:17Z

This should be now ready for final review.

RonnyPfannschmidt · 2019-10-26T16:10:42Z

doc/en/usage.rst


+.. warning::
+
+    This option is rarely used and is scheduled for removal in 5.0.


nicoddemus · 2019-10-26T16:43:04Z

Thanks @RonnyPfannschmidt

I think it is a good idea to add a "finish" entry as well including the exit code:

{"$report_type": "SessionFinish", "exitstatus": 0}

Besides including the exit status, it also allows systems to determine that pytest has finished cleanly. What do you think @RonnyPfannschmidt?

If this is to be added, I think we should rename "Header" to "SessionStart" for consistency too.

RonnyPfannschmidt · 2019-10-26T17:06:35Z

i like it

Fix pytest-dev#4488

RonnyPfannschmidt reviewed Oct 17, 2019

View reviewed changes

src/_pytest/report_log.py Show resolved Hide resolved

nicoddemus force-pushed the report-log branch 2 times, most recently from d1129cf to d8cbc62 Compare October 26, 2019 15:41

nicoddemus changed the title ~~WIP --report-log (#4488)~~ Introduce --report-log Oct 26, 2019

nicoddemus requested a review from RonnyPfannschmidt October 26, 2019 15:43

nicoddemus force-pushed the report-log branch from d8cbc62 to 46e7b0f Compare October 26, 2019 15:50

RonnyPfannschmidt reviewed Oct 26, 2019

View reviewed changes

RonnyPfannschmidt approved these changes Oct 26, 2019

View reviewed changes

nicoddemus force-pushed the report-log branch from e2a7012 to e98bf32 Compare October 30, 2019 12:42

nicoddemus added 2 commits October 30, 2019 09:43

Introduce --report-log option

b99661b

Fix pytest-dev#4488

Remove 'experimental' status from report serialization hooks

09096f7

nicoddemus force-pushed the report-log branch from e98bf32 to 09096f7 Compare October 30, 2019 12:43

nicoddemus merged commit 3406857 into pytest-dev:features Oct 30, 2019

nicoddemus deleted the report-log branch October 30, 2019 13:24

nicoddemus mentioned this pull request Oct 30, 2019

Provide an alternative to --result-log #4488

Closed

epage mentioned this pull request Nov 7, 2019

Integrate with JUnit pytest-dev/pytest-rerunfailures#97

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Introduce --report-log #5980

Introduce --report-log #5980
nicoddemus merged 2 commits intopytest-dev:featuresfrom
nicoddemus:report-log

nicoddemus commented Oct 16, 2019 •

edited

Loading

Uh oh!

RonnyPfannschmidt left a comment

Uh oh!

Uh oh!

nicoddemus commented Oct 17, 2019

Uh oh!

nicoddemus commented Oct 17, 2019

Uh oh!

RonnyPfannschmidt commented Oct 17, 2019

Uh oh!

nicoddemus commented Oct 17, 2019 •

edited

Loading

Uh oh!

RonnyPfannschmidt commented Oct 17, 2019

Uh oh!

RonnyPfannschmidt commented Oct 17, 2019

Uh oh!

nicoddemus commented Oct 26, 2019

Uh oh!

RonnyPfannschmidt Oct 26, 2019

Uh oh!

nicoddemus commented Oct 26, 2019 •

edited

Loading

Uh oh!

RonnyPfannschmidt commented Oct 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		.. warning::

		This option is rarely used and is scheduled for removal in 5.0.

Uh oh!

Conversation

nicoddemus commented Oct 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicoddemus commented Oct 17, 2019

Uh oh!

nicoddemus commented Oct 17, 2019

Uh oh!

RonnyPfannschmidt commented Oct 17, 2019

Uh oh!

nicoddemus commented Oct 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RonnyPfannschmidt commented Oct 17, 2019

Uh oh!

RonnyPfannschmidt commented Oct 17, 2019

Uh oh!

nicoddemus commented Oct 26, 2019

Uh oh!

RonnyPfannschmidt Oct 26, 2019

Choose a reason for hiding this comment

Uh oh!

nicoddemus commented Oct 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RonnyPfannschmidt commented Oct 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nicoddemus commented Oct 16, 2019 •

edited

Loading

nicoddemus commented Oct 17, 2019 •

edited

Loading

nicoddemus commented Oct 26, 2019 •

edited

Loading