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

Remove usage of funcargs #283

Merged
merged 4 commits into from
Mar 18, 2020

Conversation

christiansandberg
Copy link
Contributor

Fixes #282

ssbarnea
ssbarnea previously approved these changes Mar 16, 2020
@ssbarnea
Copy link
Member

I can confirm that this patch work fine. Can we please merge it and have a dot release?

Copy link

@gabrielecerami gabrielecerami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worked for me, thanks!

@ssbarnea
Copy link
Member

@BeyondEvil @davehunt please review.

@BeyondEvil
Copy link
Contributor

I'll try to get around to releasing this today. Whole family is sick, so I'm pretty low on energy unfortunately.

I appreciate everyones' input!

@BeyondEvil
Copy link
Contributor

BeyondEvil commented Mar 17, 2020

In an effort to avoid globals (I have a personal crusade against them), here's my suggestion:

@pytest.fixture
def extra(pytestconfig):
    """Add details to the HTML reports.

    .. code-block:: python

        import pytest_html
        def test_foo(extra):
            extra.append(pytest_html.extras.url('http://www.example.com/'))
    """
    pytestconfig.extras = []
    yield pytestconfig.extras
    del pytestconfig.extras[:]

which would turn

fixture_extras = item.funcargs.get("extra", [])

into

fixture_extras = getattr(item.config, "extras", [])

Thoughts @christiansandberg @ssbarnea @nicoddemus @RonnyPfannschmidt ?

@BeyondEvil
Copy link
Contributor

Found a bug (present in all solutions). The extra is not emptied between tests, so if test A sets an extra and is run before test B that doesn't, test B still gets the extra. Will work on a fix.

@christiansandberg
Copy link
Contributor Author

I’m also home with the kids, trying to get some work done. 🙄

Attaching it to the config is fine, although that’s also pretty much global.

Maybe you can empty the list on teardown instead?

@BeyondEvil
Copy link
Contributor

I’m also home with the kids, trying to get some work done. 🙄

I feel you! ❤️

Attaching it to the config is fine, although that’s also pretty much global.

Yes, but in a much more controlled way 😊

Maybe you can empty the list on teardown instead?

See my updated solution 😉

@christiansandberg

@BeyondEvil
Copy link
Contributor

Looks like these tests are failing on master as well. Not sure why or how.

Please confirm. If true, I'll see if I can fix it.

@BeyondEvil
Copy link
Contributor

The lastest build to pass was the release of v2.1.0

And I don't see that anything has been merged since. 🤔

@BeyondEvil
Copy link
Contributor

It's the warning causing the failure:

testing/test_pytest_html.py::TestHTML::test_css_invalid_no_html
  /Users/jimbrannlund/dev/pytest-dev/pytest-html/.tox/py37/lib/python3.7/site-packages/_pytest/terminal.py:289: PytestDeprecationWarning: TerminalReporter.writer attribute is deprecated, use TerminalReporter._tw instead at your own risk.
  See https://docs.pytest.org/en/latest/deprecations.html#terminalreporter-writer for more information.
    "TerminalReporter.writer attribute is deprecated, use TerminalReporter._tw instead at your own risk.\n"

-- Docs: https://docs.pytest.org/en/latest/warnings.html

Something must've changed in the latest release of pytest.

@BeyondEvil
Copy link
Contributor

Seems to be an issue with the pytest plugin test utility pytester.

I've logged an issue: pytest-dev/pytest#6936

@BeyondEvil
Copy link
Contributor

BeyondEvil commented Mar 18, 2020

Here's a temporary fix until the root cause is fixed:

diff --git a/testing/test_pytest_html.py b/testing/test_pytest_html.py
index 0b19c12..152fc13 100644
--- a/testing/test_pytest_html.py
+++ b/testing/test_pytest_html.py
@@ -14,7 +14,20 @@ import pytest
 pytest_plugins = ("pytester",)
 
 
+def handle_tr_writer_deprecation():
+    # Remove this function when they've fixed
+    # https://github.com/pytest-dev/pytest/issues/6936
+    import warnings
+    from _pytest.warnings import _setoption
+
+    arg = "ignore:TerminalReporter.writer:pytest.PytestDeprecationWarning"
+    _setoption(warnings, arg)
+
+
 def run(testdir, path="report.html", *args):
+    # TODO: Temporary hack until they fix
+    # https://github.com/pytest-dev/pytest/issues/6936
+    handle_tr_writer_deprecation()  # TODO: Temporary hack
     path = testdir.tmpdir.join(path)
     result = testdir.runpytest("--html", path, *args)
     return result, read_html(path)
@@ -219,6 +232,7 @@ class TestHTML:
         assert report_title in html
 
     def test_report_title_addopts_env_var(self, testdir, monkeypatch):
+        # TODO: Temporary hack until they fix
+        # https://github.com/pytest-dev/pytest/issues/6936
+        handle_tr_writer_deprecation()
         report_location = "REPORT_LOCATION"
         report_name = "MuhReport"
         monkeypatch.setenv(report_location, report_name)
@@ -878,6 +892,9 @@ class TestHTML:
         assert "No such file or directory: 'style.css'" in result.stderr.str()
 
     def test_css_invalid_no_html(self, testdir):
+        # TODO: Temporary hack until they fix
+        # https://github.com/pytest-dev/pytest/issues/6936
+        handle_tr_writer_deprecation()
         testdir.makepyfile("def test_pass(): pass")
         result = testdir.runpytest("--css", "style.css")
         assert result.ret == 0

So that we can get this merged and I can make a hotfix. @ssbarnea @christiansandberg

Copy link
Contributor

@BeyondEvil BeyondEvil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Banging job everyone! 👊

@BeyondEvil BeyondEvil merged commit 758d904 into pytest-dev:master Mar 18, 2020
@christiansandberg
Copy link
Contributor Author

Sorry for messing up. 🙏

@BeyondEvil
Copy link
Contributor

Sorry for messing up. 🙏

No worries! You fixed it, right? 😉

@ssbarnea ssbarnea added the enhancement This issue/PR relates to a feature request. label Aug 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue/PR relates to a feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2.1.0 release causing object has no attribute 'funcargs'
4 participants