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

Scottx611x/run selenium on travis only #1650

Merged
merged 7 commits into from
Apr 3, 2017
22 changes: 15 additions & 7 deletions refinery/selenium_testing/tests.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
import os

from django.contrib.auth.models import User
from django.contrib.staticfiles.testing import StaticLiveServerTestCase

from pyvirtualdisplay import Display
from selenium import webdriver
from unittest import skipIf

from core.management.commands.create_public_group import create_public_group
from core.management.commands.create_user import init_user
from core.models import Analysis, DataSet

from factory_boy.utils import make_analyses_with_single_dataset, make_datasets
from selenium_testing.utils import (
assert_body_text, login, wait_until_id_clickable, MAX_WAIT,
assert_text_within_id, delete_from_ui, wait_until_class_visible,
wait_until_id_visible)

# Start a pyvirtualdisplay for geckodriver to interact with
display = Display(visible=0, size=(1900, 1080))
display.start()
assert_body_text, assert_text_within_id, delete_from_ui, login,
MAX_WAIT, wait_until_class_visible, wait_until_id_clickable,
wait_until_id_visible
)


@skipIf("prod" not in os.getenv("DJANGO_SETTINGS_MODULE"),
Copy link
Member

Choose a reason for hiding this comment

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

As I mentioned earlier, I think this is unnecessary as we have a way to run unit tests separately from Selenium tests. This also might trip us up down the road (why are Selenium tests are not running?). I think a more flexible solution is provided in Django 1.10 (https://docs.djangoproject.com/en/1.10/topics/testing/tools/#tagging-tests).

Copy link
Member Author

Choose a reason for hiding this comment

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

@hackdna We don't currently have way to run unit tests separately from Selenium tests. Sure, you can run individual app's test suites, but this isn't a good solution, especially in the case of Refinery, because our apps are tightly coupled and changes in one app can break tests in another.

I think this solution is simple and works fine. If one wants to explicitly run selenium tests locally, switch to prod mode. Otherwise, selenium based tests are run on Travis every time you push.

As for Django 1.10, yes I mentioned that tagging feature to you... but again the ability to use that is out of our reach for awhile.

Copy link
Member

Choose a reason for hiding this comment

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

I still think that having different test runner behavior based on an environment variable is potentially confusing. Even if someone forgets to run all the tests locally, the CI system will catch that.

"We don't run selenium tests if not in production mode")
class SeleniumTestBase(StaticLiveServerTestCase):
"""Abstract base class to be used for all Selenium-based tests."""

Expand All @@ -26,6 +29,10 @@ class SeleniumTestBase(StaticLiveServerTestCase):

def setUp(self, site_login=True, initialize_guest=True,
public_group_needed=True):

# Start a pyvirtualdisplay for geckodriver to interact with
self.display = Display(visible=0, size=(1900, 1080))
Copy link
Member

Choose a reason for hiding this comment

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

Why didn't we have this before? How was the size chosen?

Copy link
Member Author

Choose a reason for hiding this comment

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

We did have this before, See here: https://github.com/refinery-platform/refinery-platform/pull/1650/files/c733de5aea8aa4871c925bf8cd17bf705758b71b#diff-f4bed93e08ec88c8c01e26197a799d9bL17

I've moved this into the setUp() and the stopping of said virtual display into tearDown()

Copy link
Member

Choose a reason for hiding this comment

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

OK, and how was the size parameter chosen?

Copy link
Member Author

Choose a reason for hiding this comment

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

@hackdna I just chose a screen resolution that I envisioned would be about the norm of what most people use nowadays. If we make the resolution too small, pages wont render properly, and tests will probably fail.

self.display.start()
self.browser = webdriver.Firefox()
self.browser.maximize_window()

Expand All @@ -42,6 +49,7 @@ def setUp(self, site_login=True, initialize_guest=True,

def tearDown(self):
self.browser.quit()
self.display.stop()

class Meta:
abstract = True
Expand Down