Change scope of live server to session #113

TWood67 · 2020-02-20T18:12:45Z

This was created from #112. I forked #63 as this had become several years out of date with master.

In addition to changes from #63, I had to skip one of the tests as I couldn't find a solution. I was unable to monkey patch the server name as monkey patch is scoped to function, and not session. Please let me know if there's a good solution to this as its currently failing.

…er defined port

TWood67 · 2020-02-20T18:14:36Z

@nicoddemus Maybe it will be best to continue the conversation here. I'm worried we won't be able to use dynamic scoping as monkey patching isn't available outside of the function scope. Any suggestions? I had to skip a test because of this.

nicoddemus · 2020-02-20T21:32:40Z

pytest_flask/fixtures.py

@@ -148,10 +151,10 @@ def test_server_is_up_and_running(live_server):
    host = pytestconfig.getvalue('live_server_host')

    # Explicitly set application ``SERVER_NAME`` for test suite
-    # and restore original value on test teardown.
    server_name = app.config['SERVER_NAME'] or 'localhost'


For this simple case we can implement the "monkeypatch" functionality ourselves:

original_server_name = app.config.get('SERVER_NAME', None) final_server_name = _rewrite_server_name(original_server_name or 'localhost', str(port)) app.config['SERVER_NAME'] = final_server_name

Then later, instead of return server, we write:

yield server # restore original value on teardown if original_server_name is not None: app.config['SERVER_NAME'] = original_server_name

Ok, this has been updated and now the test passes again. Thanks

…tion' to maintain backwards compatibility

TWood67 · 2020-02-20T22:37:05Z

@nicoddemus I was able to make the scope dynamic with my latest commit. There was confusion around this when I initially did it as the documentation is misleading, I've created this issue:
pytest-dev/pytest#6783

It looks like this functionality isn't supported by 2.7. Do you know of an easy fix? Or did you have any thoughts of dropping 2.7 support since its officially no longer maintained?

nicoddemus · 2020-02-20T23:07:03Z

I was able to make the scope dynamic with my latest commit. There was confusion around this when I initially did it as the documentation is misleading, I've created this issue:
pytest-dev/pytest#6783

Thanks for the report!

It looks like this functionality isn't supported by 2.7. Do you know of an easy fix? Or did you have any thoughts of dropping 2.7 support since its officially no longer maintained?

If 2.7 support is now a problem, then I think it is fair to drop support for it and only keep 35+ working.

I will do that in another PR.

nicoddemus · 2020-02-20T23:33:31Z

Python 2.7 support has been dropped in #114, could you please merge master into your branch? Thanks!

nicoddemus

Besides my comments, could you also please update the documentation in docs/features.rst, specially mentioning the new option and updating the examples?

Thanks again for the work on this. 👍

nicoddemus · 2020-02-20T23:34:49Z

pytest_flask/plugin.py

@@ -165,6 +165,8 @@ def pytest_addoption(parser):
                    help='use a host where to listen (default localhost).')
    group.addoption('--live-server-port', action='store', default=0, type=int,
                    help='use a fixed port for the live_server fixture.')
+    group.addoption('--live-server-scope', action='store', default='function', type=str,


As I suggested earlier, let's make this an ini option instead. 👍

The next release will be 1.0, and given that @havok2063 mentioned he has been using live_server as "session" for a long time, how about we change the default to session?

Sounds good! I wasn't sure if we wanted to support cli as well

The good thing is that you can change ini options directly in the command-line: pytest -o live_server_scope=function for example.

nicoddemus · 2020-02-20T23:35:49Z

pytest_flask/fixtures.py

-@pytest.fixture(scope='function')
-def live_server(request, app, monkeypatch, pytestconfig):
+def determine_scope(fixture_name, config):
+    if fixture_name is 'live_server':


We don't need this if given determine_scope is used only by live_server, so let's remove it:

def determine_scope(fixture_name, config): return config.getoption('--live-server-scope', 'function')

…default scope

havok2063 · 2020-02-21T16:10:14Z

I'm happy to update #63. I've been syncing this every now and then but not regularly as there has been no movement, or comments, on integrating my PR in the last few years.

TWood67 · 2020-02-21T16:52:26Z

A bit of an oversight here @nicoddemus. Changing the scope of the live server to session breaks pytest.mark.options. Due to order of instantiation, the live_server is created before we monkeypatch the app config. I think we're going to have to update the scope here as well:
https://github.com/pytest-dev/pytest-flask/blob/master/pytest_flask/plugin.py#L133

Otherwise this test will continue to fail when the scope is anything but function:
https://github.com/pytest-dev/pytest-flask/blob/master/tests/test_live_server.py#L37

I'm hoping this is the only fixture that's affected, but I'll keep digging

TWood67 · 2020-02-21T17:37:47Z

Ah yeah, looking at pytest, request is scoped to function. I hope I'm not missing something here, but I think we lose the capability of setting the flask config via options. I haven't found a need to modify these values, maybe @havok2063 has.
https://github.com/pytest-dev/pytest/blob/ef73a56032132935c377943667e155c2df4f554b/src/_pytest/fixtures.py#L355

nicoddemus · 2020-02-27T11:37:36Z

Hi @TWood67,

Ah yeah, looking at pytest, request is scoped to function

Actually request has multiple scopes, so this should be fine here.

I think we lose the capability of setting the flask config via options

You mean because of monkeypatch? Indeed monkeypatch is function-scoped only, but we might get away with patching options ourselves, like done in live_server: store the original config, set the new config, and restore the original config when the test finishes.

TWood67 · 2020-03-02T15:00:42Z

@nicoddemus Thanks for all your help, tests are passing now

nicoddemus · 2020-03-03T11:45:24Z

Thanks a ton @TWood67 and @havok2063! I've squased and merged the changes, keeping a Co-authored-by field for @havok2063. 👍

This was brought up in pytest-dev#113, but was missed.

nicoddemus · 2020-03-03T12:11:11Z

Hey @TWood67, while preparing the release I noticed you probably missed #113 (comment), so I changed it to an ini option in #116. 👍

havok2063 and others added 5 commits April 7, 2017 11:05

changed live_server fixture to be a session scoped and to accept a us…

4d5026c

…er defined port

changing app in conftest to session scope

0937483

merged upstream master into this

cb25558

fixed test to have a scope session

186291e

skipped a test because we can't monkey patch the item

d901965

nicoddemus reviewed Feb 20, 2020

View reviewed changes

added an option to configure the live server scope. defaults to 'func…

a156249

…tion' to maintain backwards compatibility

nicoddemus requested changes Feb 20, 2020

View reviewed changes

TWood67 added 2 commits February 21, 2020 09:52

Merge branch 'master' of github.com:pytest-dev/pytest-flask

d865348

modified determine scope to not check fixture name. Made session the …

38d94d0

…default scope

updated docs

62d05b6

modified scope of rewrite test to function

3a58118

nicoddemus merged commit 5941e8e into pytest-dev:master Mar 3, 2020

nicoddemus added a commit to nicoddemus/pytest-flask that referenced this pull request Mar 3, 2020

Change --live-server-scope to an ini option

7fbc736

This was brought up in pytest-dev#113, but was missed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change scope of live server to session #113

Change scope of live server to session #113

TWood67 commented Feb 20, 2020

TWood67 commented Feb 20, 2020

nicoddemus Feb 20, 2020

TWood67 Feb 20, 2020

TWood67 commented Feb 20, 2020

nicoddemus commented Feb 20, 2020

nicoddemus commented Feb 20, 2020

nicoddemus left a comment

nicoddemus Feb 20, 2020

nicoddemus Feb 20, 2020

TWood67 Feb 21, 2020

nicoddemus Feb 21, 2020

nicoddemus Feb 20, 2020

havok2063 commented Feb 21, 2020

TWood67 commented Feb 21, 2020 •

edited

TWood67 commented Feb 21, 2020

nicoddemus commented Feb 27, 2020

TWood67 commented Mar 2, 2020

nicoddemus commented Mar 3, 2020

nicoddemus commented Mar 3, 2020

Change scope of live server to session #113

Change scope of live server to session #113

Conversation

TWood67 commented Feb 20, 2020

TWood67 commented Feb 20, 2020

nicoddemus Feb 20, 2020

Choose a reason for hiding this comment

TWood67 Feb 20, 2020

Choose a reason for hiding this comment

TWood67 commented Feb 20, 2020

nicoddemus commented Feb 20, 2020

nicoddemus commented Feb 20, 2020

nicoddemus left a comment

Choose a reason for hiding this comment

nicoddemus Feb 20, 2020

Choose a reason for hiding this comment

nicoddemus Feb 20, 2020

Choose a reason for hiding this comment

TWood67 Feb 21, 2020

Choose a reason for hiding this comment

nicoddemus Feb 21, 2020

Choose a reason for hiding this comment

nicoddemus Feb 20, 2020

Choose a reason for hiding this comment

havok2063 commented Feb 21, 2020

TWood67 commented Feb 21, 2020 • edited

TWood67 commented Feb 21, 2020

nicoddemus commented Feb 27, 2020

TWood67 commented Mar 2, 2020

nicoddemus commented Mar 3, 2020

nicoddemus commented Mar 3, 2020

TWood67 commented Feb 21, 2020 •

edited