From 734d33fc279d4d22229db56c304f6cf7141e3e52 Mon Sep 17 00:00:00 2001 From: bjones1 Date: Wed, 17 Mar 2021 22:48:13 +0100 Subject: [PATCH 1/6] Update: Run NPM build from pytest. Del: Unused Travis file. --- .github/workflows/python-package.yml | 1 - .travis.yml | 24 ------------------------ 2 files changed, 25 deletions(-) delete mode 100644 .travis.yml diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index c7e58362c..088fe4b70 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -31,7 +31,6 @@ jobs: id: create-runestone-bundle run: | npm install - npm run build - name: Set up Python ${{ matrix.python-version }} id: install-python uses: actions/setup-python@v2 diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 5e54de44e..000000000 --- a/.travis.yml +++ /dev/null @@ -1,24 +0,0 @@ -# ********* -# |docname| -# ********* -dist: xenial -language: python -python: -- '3.7' -- '3.8' -node_js: "13.8" -install: "npm install" - -script: -- export LANG=en_US.UTF-8 -- wget https://chromedriver.storage.googleapis.com/83.0.4103.39/chromedriver_linux64.zip -- unzip chromedriver_linux64.zip -- sudo apt-get -y update -- sudo apt-get -y install google-chrome-stable -- export PATH=$TRAVIS_BUILD_DIR:$PATH -- "python -m pip install -U pip" -- "pip install -e ." -- "pip install -U -r requirements-dev.txt" -- "pytest" -notifications: - slack: runestoneteam:aKSajF6qfp9WpgisFH8mocs6 From 1a63f4b2abb3daeb7a08c3fddf3250bc7a404b8c Mon Sep 17 00:00:00 2001 From: bjones1 Date: Thu, 18 Mar 2021 00:32:55 +0100 Subject: [PATCH 2/6] Test: Move to shared tests between the components and the server. --- runestone/conftest.py | 42 +++++++++++++ runestone/shared_conftest.py | 110 +++++++++++++++++++++++++++++++++++ runestone/unittest_base.py | 44 ++++---------- 3 files changed, 162 insertions(+), 34 deletions(-) create mode 100644 runestone/conftest.py create mode 100644 runestone/shared_conftest.py diff --git a/runestone/conftest.py b/runestone/conftest.py new file mode 100644 index 000000000..64411a6c9 --- /dev/null +++ b/runestone/conftest.py @@ -0,0 +1,42 @@ +# *************************************** +# |docname| - pytest fixtures for testing +# *************************************** +# This defines fixtures specific to the client for test. These same fixtures are defined differently on the server to accommodate the different setup these. +# +# TODO: rewrite the tests to use pytest-native syntax, then put the contents of ``unittest_base`` here. +# +# Imports +# ======= +# These are listed in the order prescribed by `PEP 8 +# `_. +# +# Standard library +# ---------------- +# None. +# +# Third-party imports +# ------------------- +import pytest + + +# Local imports +# ------------- +# This is necessary to bring in the shared pytest fixture. +from runestone.shared_conftest import _SeleniumUtils, selenium_driver # noqa: F401 +from runestone.unittest_base import ModuleFixture, HOST_URL, run_webpack # noqa: F401 + + +# Fixtures +# ======== +@pytest.fixture(scope="module") +def selenium_driver_session(request): + mf = ModuleFixture(request.fspath, True) + mf.setUpModule() + yield mf.driver + mf.tearDownModule() + + +# Present ``_SeleniumUser`` as a fixture. To use, provide it with a ``_TestUser`` instance. +@pytest.fixture +def selenium_utils(selenium_driver): # noqa: F811 + return _SeleniumUtils(selenium_driver, HOST_URL) diff --git a/runestone/shared_conftest.py b/runestone/shared_conftest.py new file mode 100644 index 000000000..f8e5dc865 --- /dev/null +++ b/runestone/shared_conftest.py @@ -0,0 +1,110 @@ +# ************************************************************** +# |docname| - pytest fixtures used by both the client and server +# ************************************************************** +# The goal of this test framework is to enable tests written for the Runestone Components to be easily run on the server, where values stored by the database can also be checked. This avoids a duplication of test code between Runestone Components and the server. +# +# To accomplish this goal, this file provides functions that work on both client and server. Then, the client and server must each uniquely define two fixtures (see `conftest.py`) specalized for that environment which provided the same results (access to the Selenium WebDriver and to a class of Selenium utilities). Both client and server tests must import from this file; tests can then +# +# Structure: +# +# - The ``selenium_driver_session`` fixture performs client- and server-specific setup, then returns an instance of a Selenium WebDriver. Since it's specific to the environment, this fixture must be defined differently for client and server. It should be defined at the module level or higher, since starting Selenium is an expensive operation. +# - The shared `selenium_driver`_ fixture then sets up/tears down the Selenium environment for each test. +# - The client/server-specific ``selenium_utils`` fixture wraps the shared `_SeleniumUtils`_ class, which provides a common set of methods for the tests. +# - All client tests should use the ``selenium_utils`` fixture, ensuring that they can run in either environment. +# +# Imports +# ======= +# These are listed in the order prescribed by `PEP 8 +# `_. +# +# Standard library +# ---------------- +# None. +# +# Third-party imports +# ------------------- +import pytest +from selenium.common.exceptions import StaleElementReferenceException +from selenium.webdriver.common.by import By +from selenium.webdriver.support.ui import WebDriverWait + +# Local imports +# ------------- +# None. + + +# Code +# ==== +# +# selenium_driver +# --------------- +# Provide a way to reset the state of Selenium for each test, without exiting/restarting the driver (which is slow). +@pytest.fixture() +def selenium_driver(selenium_driver_session): + driver = selenium_driver_session + # Copied from the Runestone Components test framework. + driver.implicitly_wait(10) + + yield driver + + # Clear as much as possible, to present an almost-fresh instance of a browser for the next test. (Shutting down then starting up a browser is very slow.) + driver.execute_script("window.localStorage.clear();") + driver.execute_script("window.sessionStorage.clear();") + driver.delete_all_cookies() + + +# _SeleniumUtils +# -------------- +# Provide basic Selenium-based operations. +class _SeleniumUtils: + def __init__( + self, + # These are fixtures. + runestone_selenium_driver, + host_address, + ): + + self.driver = runestone_selenium_driver + self.host_address = host_address + self.wait = WebDriverWait(self.driver, 10) + + # A helper function to attach to the Selenium driver: get from a URL relative to the Runestone application. + def get(self, relative_url): + return self.driver.get( + "{}/{}".format(self.host_address, relative_url) + ) + + # Wait until a Runestone component has finished rendering itself, given the ID of the component. + def wait_until_ready(self, id): + # The component is ready when it has the class below. + self.wait.until( + element_has_css_class((By.ID, id), "runestone-component-ready") + ) + + +# An expectation for Selenium, used for checking that an element has a particular css class. From the `Selenium docs `_, under the "Custom wait conditions" subheading. +# +# locator - used to find the element +# +# returns the WebElement once it has the particular css class. +class element_has_css_class: + def __init__( + self, + # The element to find; this is passed directly to `driver.find_element `_. See the `Selenium docs`_. + locator, + # The CSS class to look for. + css_class, + ): + + self.locator = locator + self.css_class = css_class + + def __call__(self, driver): + # Find the referenced element. Ignore stale elements. + try: + element = driver.find_element(*self.locator) + if self.css_class in element.get_attribute("class"): + return element + except StaleElementReferenceException: + pass + return False diff --git a/runestone/unittest_base.py b/runestone/unittest_base.py index 460d2dd7f..9467b421c 100644 --- a/runestone/unittest_base.py +++ b/runestone/unittest_base.py @@ -25,17 +25,16 @@ import pytest from pyvirtualdisplay import Display from selenium import webdriver -from selenium.common.exceptions import StaleElementReferenceException from selenium.webdriver.chrome.options import Options from selenium.webdriver.common.by import By from selenium.webdriver.support.ui import WebDriverWait -logging.basicConfig(level=logging.WARN) -mylogger = logging.getLogger() - # Local imports # ------------- -# None +from runestone.shared_conftest import element_has_css_class, _SeleniumUtils + +logging.basicConfig(level=logging.WARN) +mylogger = logging.getLogger() # Globals # ======= @@ -59,7 +58,8 @@ # Run this once, before all tests, to update the webpacked JS. @pytest.fixture(scope="session", autouse=True) def run_webpack(): - subprocess.run(["npm", "run", "build"], check=True) + # Note that Windows requires ``shell=True``, since the command to exeucute is ``npm.cmd``. + subprocess.run(["npm", "run", "build"], check=True, shell=IS_WINDOWS) # Define `module fixtures `_ to build the test Runestone project, run the server, then shut it down when the tests complete. @@ -229,6 +229,10 @@ def wait_until_ready(self, id): element_has_css_class((By.ID, id), "runestone-component-ready") ) + # Get a page from the local server. + def get(self, relative_url): + return self.driver.get(f"{self.host}/{relative_url}") + def setUp(self): # Use the shared module-wide driver. self.driver = mf.driver @@ -244,31 +248,3 @@ def tearDown(self): self.driver.execute_script("window.localStorage.clear();") self.driver.execute_script("window.sessionStorage.clear();") self.driver.delete_all_cookies() - - -# An expectation for Selenium, used for checking that an element has a particular css class. From the `Selenium docs `_, under the "Custom wait conditions" subheading. -# -# locator - used to find the element -# -# returns the WebElement once it has the particular css class. -class element_has_css_class: - def __init__( - self, - # The element to find; this is passed directly to `driver.find_element `_. See the `Selenium docs`_. - locator, - # The CSS class to look for. - css_class, - ): - - self.locator = locator - self.css_class = css_class - - def __call__(self, driver): - # Find the referenced element. Ignore stale elements. - try: - element = driver.find_element(*self.locator) - if self.css_class in element.get_attribute("class"): - return element - except StaleElementReferenceException: - pass - return False From 6993e64fb033deffd24a211bd98b13a75b500ad4 Mon Sep 17 00:00:00 2001 From: bjones1 Date: Wed, 17 Mar 2021 22:49:56 +0100 Subject: [PATCH 3/6] Fix: Rewrite poll to use new testing framework. --- runestone/common/js/runestonebase.js | 29 ++++++++++++++---- runestone/poll/js/poll.js | 10 +++++-- runestone/poll/test/test_poll.py | 45 +++++++++++----------------- 3 files changed, 48 insertions(+), 36 deletions(-) diff --git a/runestone/common/js/runestonebase.js b/runestone/common/js/runestonebase.js index 994e8e5ef..d4d1a36cf 100644 --- a/runestone/common/js/runestonebase.js +++ b/runestone/common/js/runestonebase.js @@ -47,7 +47,7 @@ export default class RunestoneBase { } // This is for the selectquestion points // If a selectquestion is part of a timed exam it will get - // the timedWrapper options. + // the timedWrapper options. if (typeof opts.timedWrapper !== "undefined") { this.timedWrapper = opts.timedWrapper; } else { @@ -158,13 +158,20 @@ export default class RunestoneBase { } return post_promise; } - /* Checking/loading from storage + /* Checking/loading from storage **WARNING:** DO NOT `await` this function! - This function, although async, does not explicitly resolve its promise by returning a value. The reason for this is because it is called by the constructor for nearly every component. In Javascript constructors cannot be async! - + This function, although async, does not explicitly resolve its promise by returning a value. The reason for this is because it is called by the constructor for nearly every component. In Javascript constructors cannot be async! + One of the recommended ways to handle the async requirements from within a constructor is to use an attribute as a promise and resolve that attribute at the appropriate time. */ - async checkServer(eventInfo) { + async checkServer( + // A string specifying the event name to use for querying the :ref:`getAssessResults` endpoint. + eventInfo, + // If true, this function will invoke ``indicate_component_ready()`` just before it returns. This is provided since most components are ready after this function completes its work. + // + // TODO: This defaults to false, to avoid causing problems with any components that haven't been updated and tested. After all Runestone components have been updated, default this to true and remove the extra parameter from most calls to this function. + will_be_ready = false + ) { // Check if the server has stored answer let self = this; this.checkServerComplete = new Promise(function (resolve, reject) { @@ -212,6 +219,18 @@ export default class RunestoneBase { this.checkLocalStorage(); // just go right to local storage this.csresolver("local"); } + + if (will_be_ready) { + this.indicate_component_ready(); + } + } + + // This method assumes that ``this.componentDiv`` refers to the ``div`` containing the component, and that this component's ID is set. + indicate_component_ready() { + // Add a class to indicate the component is now ready. + this.containerDiv.classList.add("runestone-component-ready"); + // Produce an event as well. + $(document).trigger("runestone:component-ready", this.containerDiv.id); } loadData(data) { diff --git a/runestone/poll/js/poll.js b/runestone/poll/js/poll.js index f81380d71..319991bb0 100644 --- a/runestone/poll/js/poll.js +++ b/runestone/poll/js/poll.js @@ -25,7 +25,8 @@ export default class Poll extends RunestoneBase { this.getQuestionText(); this.getOptionText(); //populates optionList this.renderPoll(); //generates HTML - this.checkPollStorage(); //checks localStorage to see if this poll has already been completed by this user + // Checks localStorage to see if this poll has already been completed by this user. + this.checkPollStorage(); this.caption = "Poll"; this.addCaption("runestone"); } @@ -59,7 +60,7 @@ export default class Poll extends RunestoneBase { this.containerDiv = document.createElement("div"); this.pollForm = document.createElement("form"); this.resultsDiv = document.createElement("div"); - this.containerDiv.id = this.divid + "_container"; + this.containerDiv.id = this.divid; $(this.containerDiv).addClass(this.origElem.getAttribute("class")); $(this.pollForm).html( `${this.question}` @@ -212,6 +213,7 @@ export default class Poll extends RunestoneBase { } $(this.resultsDiv).append(list); } + this.indicate_component_ready(); } disableOptions() {} checkPollStorage() { @@ -227,7 +229,9 @@ export default class Poll extends RunestoneBase { eBookConfig.ajaxURL + "getpollresults", data, this.showPollResults.bind(this) - ); + ).fail(this.indicate_component_ready.bind(this)); + } else { + this.indicate_component_ready(); } } } diff --git a/runestone/poll/test/test_poll.py b/runestone/poll/test/test_poll.py index aadd85d87..2bd8067ba 100644 --- a/runestone/poll/test/test_poll.py +++ b/runestone/poll/test/test_poll.py @@ -1,36 +1,25 @@ -from runestone.unittest_base import module_fixture_maker, RunestoneTestCase -from selenium.webdriver.support import expected_conditions as EC -from selenium.webdriver.support.ui import WebDriverWait -from selenium.webdriver.common.by import By +def _test_poll(selenium_utils, id): + """ test the poll directive """ + self = selenium_utils + self.wait_until_ready(id) + poll_div = self.driver.find_element_by_id(id) -setUpModule, tearDownModule = module_fixture_maker(__file__) + opts = poll_div.find_elements_by_css_selector("input[type='radio']") + # the poll in overview should be on a scale 1-10. + assert len(opts) == 10, "Not enough poll options present!" -class PollTests(RunestoneTestCase): - def test_poll(self): - """ test the poll directive """ - self.driver.get(self.host + "/index.html") - wait = WebDriverWait(self.driver, 10) - try: - wait.until(EC.presence_of_element_located((By.ID, "pollid1_container"))) - except: - text = self.driver.page_source - print(text[:300]) + # just choose option 4 + poll_div.find_element_by_id(f"{id}_opt_4").click() - poll_div = self.driver.find_element_by_id("pollid1_container") + el = poll_div.find_element_by_id(f"{id}_sent") # check for results span + assert el.text[:6] == "Thanks" - opts = poll_div.find_elements_by_css_selector("input[type='radio']") + # just make sure we can find the results div - an exception will be raised if the div cannot be found + poll_div.find_element_by_id(f"{id}_results") - # the poll in overview should be on a scale 1-10. - self.assertTrue(len(opts) == 10, "Not enough poll options present!") - # just choose option 4 - poll_div.find_element_by_id("pollid1_opt_4").click() - - el = poll_div.find_element_by_id("pollid1_sent") # check for results span - self.assertIsNotNone(el) - self.assertEqual(el.text[:6], "Thanks") - - # just make sure we can find the results div - an exception will be raised if the div cannot be found - poll_div.find_element_by_id("pollid1_results") +def test_poll(selenium_utils): + selenium_utils.get("index.html") + _test_poll(selenium_utils, "pollid1") From d4e112a62c561296e253df5685f8c35378e53f12 Mon Sep 17 00:00:00 2001 From: bjones1 Date: Thu, 18 Mar 2021 00:57:36 +0100 Subject: [PATCH 4/6] Fix: Unit test run of npm doesn't work on Github. --- .github/workflows/python-package.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 088fe4b70..c7e58362c 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -31,6 +31,7 @@ jobs: id: create-runestone-bundle run: | npm install + npm run build - name: Set up Python ${{ matrix.python-version }} id: install-python uses: actions/setup-python@v2 From 646d3611564ddd6d1e4fe76213a4f6f243eb9eb1 Mon Sep 17 00:00:00 2001 From: bjones1 Date: Thu, 18 Mar 2021 13:38:17 +0100 Subject: [PATCH 5/6] Change: Resolve a promise instead of emitting an event when a component is ready. --- runestone/common/js/runestonebase.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/runestone/common/js/runestonebase.js b/runestone/common/js/runestonebase.js index d4d1a36cf..619e8474e 100644 --- a/runestone/common/js/runestonebase.js +++ b/runestone/common/js/runestonebase.js @@ -20,6 +20,7 @@ import { pageProgressTracker } from "./bookfuncs.js"; export default class RunestoneBase { constructor(opts) { + this.component_ready_promise = new Promise(resolve => this._component_ready_resolve_fn = resolve) this.optional = false; if (opts) { this.sid = opts.sid; @@ -229,8 +230,8 @@ export default class RunestoneBase { indicate_component_ready() { // Add a class to indicate the component is now ready. this.containerDiv.classList.add("runestone-component-ready"); - // Produce an event as well. - $(document).trigger("runestone:component-ready", this.containerDiv.id); + // Resolve the ``this.component_ready_promise``. + this._component_ready_resolve_fn(); } loadData(data) { From 064885018703c98858b63ac723ed0c4a3d5e41cc Mon Sep 17 00:00:00 2001 From: bjones1 Date: Thu, 18 Mar 2021 14:30:48 +0100 Subject: [PATCH 6/6] Fix: Manually tweak timing to make tests pass. --- runestone/shortanswer/test/test_shortanswer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runestone/shortanswer/test/test_shortanswer.py b/runestone/shortanswer/test/test_shortanswer.py index 6d2b2c100..99dd4b6ec 100644 --- a/runestone/shortanswer/test/test_shortanswer.py +++ b/runestone/shortanswer/test/test_shortanswer.py @@ -24,7 +24,7 @@ def test_sa1(self): def test_sa2(self): """No input. Button clicked""" self.driver.get(self.host + "/index.html") - sleep(1) + sleep(2) t1 = self.driver.find_element_by_id("question1") btn_check = t1.find_element_by_tag_name("button")