Scottx611x/run selenium on travis only #1650
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1650 +/- ##
===========================================
+ Coverage 37.03% 38.65% +1.61%
===========================================
Files 369 369
Lines 24093 25007 +914
Branches 1260 1260
===========================================
+ Hits 8924 9667 +743
- Misses 15169 15340 +171
Continue to review full report at Codecov.
|
…ub.com/refinery-platform/refinery-platform into scottx611x/run_selenium_on_travis_only # Fixed Conflicts: # refinery/selenium_testing/tests.py
refinery/selenium_testing/tests.py
Outdated
| @@ -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)) | |||
There was a problem hiding this comment.
Why didn't we have this before? How was the size chosen?
There was a problem hiding this comment.
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()
There was a problem hiding this comment.
OK, and how was the size parameter chosen?
There was a problem hiding this comment.
@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.
refinery/selenium_testing/tests.py
Outdated
|
|
||
|
|
||
| @skipIf("prod" not in os.getenv("DJANGO_SETTINGS_MODULE"), |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
No description provided.