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

Live server subprocess #47

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fuhrysteve
Copy link

@fuhrysteve fuhrysteve commented Jul 24, 2016

Please don't merge this yet. There is still one test that is not passing for a mysterious reason, and additionally this code still needs test cases of its own.

I'm working on a way to launch flask as a completely separate process using os.fork by way of watcher_getter (from pytest-services).

The reason I need to use this strategy is that SQLAlchemy configurations become considerably more complex when you have to deal with multiprocessing (see docs). I was struggling to get it to work in my existing configuration, and this seemed like the easier option.

Let me know if you have any initial feedback and I'll circle back. Thanks @vitalk!

@vitalk
Copy link
Collaborator

vitalk commented Jul 24, 2016

Hi @fuhrysteve! This changes looks good for me. But live_server_subprocess fixture can be always available (not only when pytest-service is exists).

It would be great if live_server_subprocess fixture can detect missed requirements as well (such as flask>=0.11.0 or pytest-service) and skip tests with helpful message.

@@ -34,35 +36,38 @@ def login(self, email, password):
return self.client.post(url_for('login'), data=credentials)

def test_login(self):
assert self.login('vital@example.com', 'pass').status_code == 200
assert self.login('vital@foo.com', 'pass').status_code == 200
Copy link
Collaborator

Choose a reason for hiding this comment

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

Example.com, example.net, example.org, and example.edu are second-level domain names reserved for documentation purposes and examples of the use of domain names.

Copy link
Author

Choose a reason for hiding this comment

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

I'll change it back - I was just trying to get the line to squeeze into 80 chars so my pep8 checker would stop complaining :)

@vitalk
Copy link
Collaborator

vitalk commented Jul 24, 2016

Use the separate --start-live-server-subprocess option to run live_server fixture as subprocess is another suggestion. Any thoughts?

@fuhrysteve
Copy link
Author

fuhrysteve commented Jul 24, 2016

@vitalk So I think the issue with always making live_server_subprocess always available is I'm not sure off the top of my head how to do that without making all tests fail at collection time - the reason is that watcher_getter would fail because it says it can't find a fixture with that name.

So I'm not sure how to make it optionally available unless there's some way I'm not aware of to get a fixture at runtime.

Any suggestions?

@fuhrysteve
Copy link
Author

@vitalk with regards to --start-live-server-subprocess, sounds like a good idea to me - i'll add it in.

@vitalk
Copy link
Collaborator

vitalk commented Jul 24, 2016

the reason is that watcher_getter would fail because it says it can't find a fixture with that name

I missed that part, maybe I can find a workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants