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

Introduce new hook to better control display of items in terminal #5047

Open
nicoddemus opened this issue Apr 4, 2019 · 9 comments
Open
Labels
type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature

Comments

@nicoddemus
Copy link
Member

Recently I have added TestReport.count_towards_summary and TestReport.head_line properties as a way to control how items are shown/handled by the terminal plugin. After some feedback it is clear that I would need to customize other things as well.

The current approach of adding a property to TestReport so it can be overwritten by subclasses in plugins is sub-optimal: it requires access to the private TestReport class to declare it in the bases list of the subclass, and subclasses in general are not a good way to extend plugins functionality in plugins traditionally as they expose too much internal details.

I propose then that we add a new hook:

@hookspec(firstresult=True)
def pytest_report_displayopts(report, config) -> dict:
    """
    (Experimental)

    Returns a number of options related to the given report object. This is used
    by the terminal writer and other plugins to customize how some visual aspects 
    of tests are shown.
    
    This hook returns a dict, which may contain any key and value. Currently the 
    following keys are recognized by pytest:


    * ``category``: same as returned by ``pytest_report_teststatus``
    * ``letter``: same as returned by ``pytest_report_teststatus``
    * ``word``: same as returned by ``pytest_report_teststatus``
    * ``show-progress``: bool, should show progress at right margin or not
    * ``head-line``: the head line shown with longrepr output for this report, more commonly during
        traceback representation during failures.
    * ``count-towards-summary``: True if this report should be counted towards the totals shown at the end of the
        test session: "1 passed, 1 failure, etc".
    * ``nodeid-caption``: how to show the nodeid when in verbose mode; usually this is the
        bare nodeid, but plugins might add more information such as subtest context (with
        ``pytest-subtests``), repeat count (``pytest-repeat``), number of retries (``flaky`` plugin),
        or workers where the test was executed (``pytest-xdist``).

    In the interest of forward and backward compatibility, extra keys are ignored, so new keys can be
    added in the future and plugins which use those keys will still work in older pytest versions.
    """ 

A big advantage here is that it is easy to extend the customizability of how reports are displayed in the
future, without introducing new hooks or breaking backward compatibility. Also it will allow us to get rid of
xdist-specific code in the terminal plugin, as we will be able to delegate the decisions to the hook.

Thoughts?

cc @RonnyPfannschmidt

@nicoddemus nicoddemus added the type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature label Apr 4, 2019
@blueyed
Copy link
Contributor

blueyed commented Apr 4, 2019

Sounds good!

@RonnyPfannschmidt
Copy link
Member

Could use a more specific name, terminalwriter options and more general options need a sane way to communicate

In particular for other ux systems (im hoping for eventually a html5/electron ux integrated with looponfail, xdist and tox

@nicoddemus
Copy link
Member Author

@RonnyPfannschmidt I thought about using "terminal" in the name, but realized that other plugins will want to call this hook, just from the top of my head junitxml and pytest-html; they will be interested to use nodeid_caption probably as it will bring more context to the test items.

I'm open to other naming suggestions, but "terminal" specifically doesn't seem like a good fit for the reasons mentioned above.

@RonnyPfannschmidt
Copy link
Member

@nicoddemus we basically have something that begs for polymorphism, multi-methods or generic methods, any easy solution will have weird edges

@nicoddemus
Copy link
Member Author

@RonnyPfannschmidt can you elaborate? It is not clear from your answer if it is a suggestion, a 👎 for my proposal, or just musings for the future. 😁

@RonnyPfannschmidt
Copy link
Member

Future musing and a -0

@nicoddemus
Copy link
Member Author

OK, thanks.

I plan to go ahead with this anyway given you are not a strong -1; I believe it will help cleanup the terminal code off the xdist-specific code, and provide more flexibility for existing plugins.

Anyway I will mark it off as experimental, so that a limited number of plugins and the core should use it and provide feedback.

@blueyed
Copy link
Contributor

blueyed commented Apr 15, 2019

With regard to #5122 I think it might make sense to have these as properties/methods on the BaseReport also, which allows to customize/override it from there.
The report instance would then fall back to using this hook, and having it there allows for proper caching (i.e. call the hook only once per report).

(My current use case is adjusting the verbose name (word: same as returned by pytest_report_teststatus) for the short test summary, but a firstresult hook does not allow for appending, does it?)

@blueyed
Copy link
Contributor

blueyed commented Oct 21, 2019

Something similar might make sense also with "displaying objects" in assertion rewriting / tb-locals etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: proposal proposal for a new feature, often to gather opinions or design the API around the new feature
Projects
None yet
Development

No branches or pull requests

3 participants