CI Test Fixes Round 3 (#216)

* Refactors for saving and related utils updates.

* Revised/fixed docstring.

nbclassic/tests/end_to_end/test_save_as_notebook.py

@@ -1,42 +1,122 @@
-"""Test readonly notebook saved and renamed"""
+"""Test save-as functionality"""
 
 
-from .utils import EDITOR_PAGE
-from playwright.sync_api import sync_playwright, TimeoutError as PlaywrightTimeoutError
+import traceback
+
+from .utils import EDITOR_PAGE, EndToEndTimeout
 
 
 def save_as(nb):
     JS = '() => Jupyter.notebook.save_notebook_as()'
     return nb.evaluate(JS, page=EDITOR_PAGE)
 
+
 def get_notebook_name(nb):
     JS = '() => Jupyter.notebook.notebook_name'
     return nb.evaluate(JS, page=EDITOR_PAGE)
 
+
 def set_notebook_name(nb, name):
     JS = f'() => Jupyter.notebook.rename("{name}")'
     nb.evaluate(JS, page=EDITOR_PAGE)
 
 
-def test_save_notebook_as(notebook_frontend):
-    set_notebook_name(notebook_frontend, name="nb1.ipynb")
+def test_save_as_nb(notebook_frontend):
+    print('[Test] [test_save_readonly_as]')
+    notebook_frontend.wait_for_kernel_ready()
+    notebook_frontend.wait_for_selector(".input", page=EDITOR_PAGE)
 
-    notebook_frontend.locate('#notebook_name', page=EDITOR_PAGE)
-
-    assert get_notebook_name(notebook_frontend) == "nb1.ipynb"
+    # Set a name for comparison later
+    print('[Test] Set notebook name')
+    set_notebook_name(notebook_frontend, name="nb1.ipynb")
+    notebook_frontend.wait_for_condition(
+        lambda: get_notebook_name(notebook_frontend) == 'nb1.ipynb',
+        timeout=150,
+        period=1
+    )
 
     # Wait for Save As modal, save
+    print('[Test] Save')
     save_as(notebook_frontend)
-    notebook_frontend.wait_for_selector('.save-message', page=EDITOR_PAGE)
 
-    # TODO: Add a function for locator assertions to FrontendElement
-    locator_element = notebook_frontend.locate_and_focus('//input[@data-testid="save-as"]', page=EDITOR_PAGE)
-    locator_element.wait_for('visible')
+    # # Wait for modal to pop up
+    print('[Test] Waiting for modal popup')
+    notebook_frontend.wait_for_selector(".modal-footer", page=EDITOR_PAGE)
+    dialog_element = notebook_frontend.locate(".modal-footer", page=EDITOR_PAGE)
+    dialog_element.focus()
+
+    print('[Test] Focus the notebook name input field, then click and modify its .value')
+    notebook_frontend.wait_for_selector('.modal-body .form-control', page=EDITOR_PAGE)
+    name_input_element = notebook_frontend.locate('.modal-body .form-control', page=EDITOR_PAGE)
+    name_input_element.focus()
+    name_input_element.click()
+    notebook_name = 'new_notebook.ipynb'
+
+    print('[Test] Begin attempts to fill the save dialog input and save the notebook')
+    fill_attempts = 0
+
+    def attempt_form_fill_and_save():
+        # Application behavior here is HIGHLY variable, we use this for repeated attempts
+        # ....................
+        # This may be a retry, check if the application state reflects a successful save operation
+        nonlocal fill_attempts
+        if fill_attempts and get_notebook_name(notebook_frontend) == "new_notebook.ipynb":
+            print('[Test]   Success from previous save attempt!')
+            return True
+        fill_attempts += 1
+        print(f'[Test] Attempt form fill and save #{fill_attempts}')
+
+        # Make sure the save prompt is visible
+        if not name_input_element.is_visible():
+            save_as(notebook_frontend)
+            name_input_element.wait_for('visible')
+
+        # Set the notebook name field in the save dialog
+        print('[Test] Fill the input field')
+        name_input_element.evaluate(f'(elem) => {{ elem.value = "new_notebook.ipynb"; return elem.value; }}')
+        notebook_frontend.wait_for_condition(
+            lambda: name_input_element.evaluate(
+                f'(elem) => {{ elem.value = "new_notebook.ipynb"; return elem.value; }}') == 'new_notebook.ipynb',
+            timeout=120,
+            period=.25
+        )
+        # Show the input field value
+        print('[Test] Name input field contents:')
+        field_value = name_input_element.evaluate(f'(elem) => {{ return elem.value; }}')
+        print('[Test]   ' + field_value)
+        if field_value != 'new_notebook.ipynb':
+            return False
+
+        print('[Test] Locate and click the save button')
+        save_element = dialog_element.locate('text=Save')
+        save_element.wait_for('visible')
+        save_element.focus()
+        save_element.click()
+
+        # Application lag may cause the save dialog to linger,
+        # if it's visible wait for it to disappear before proceeding
+        if save_element.is_visible():
+            print('[Test] Save element still visible after save, wait for hidden')
+            try:
+                save_element.expect_not_to_be_visible(timeout=120)
+            except EndToEndTimeout as err:
+                traceback.print_exc()
+                print('[Test]   Save button failed to hide...')
+
+        # Check if the save operation succeeded (by checking notebook name change)
+        notebook_frontend.wait_for_condition(
+            lambda: get_notebook_name(notebook_frontend) == "new_notebook.ipynb", timeout=120, period=5
+        )
+        print(f'[Test] Notebook name: {get_notebook_name(notebook_frontend)}')
+        print('[Test] Notebook name was changed!')
+        return True
 
-    notebook_frontend.insert_text('new_notebook.ipynb', page=EDITOR_PAGE)
-    notebook_frontend.try_click_selector('//html//body//div[8]//div//div//div[3]//button[2]', page=EDITOR_PAGE)
-
-    locator_element.expect_not_to_be_visible()
+    # Retry until timeout (wait_for_condition retries upon func exception)
+    notebook_frontend.wait_for_condition(attempt_form_fill_and_save, timeout=900, period=1)
 
-    assert get_notebook_name(notebook_frontend) == "new_notebook.ipynb"
-    assert "new_notebook.ipynb" in notebook_frontend.get_page_url(page=EDITOR_PAGE)
+    print('[Test] Check notebook name in URL')
+    notebook_frontend.wait_for_condition(
+        lambda: notebook_name in notebook_frontend.get_page_url(page=EDITOR_PAGE),
+        timeout=120,
+        period=5
+    )

nbclassic/tests/end_to_end/test_save_readonly_as.py

@@ -3,8 +3,7 @@
 
 import traceback
 
-from .utils import EDITOR_PAGE, TimeoutError as TestingTimeout
-from playwright.sync_api import sync_playwright, TimeoutError as PlaywrightTimeoutError
+from .utils import EDITOR_PAGE, EndToEndTimeout
 
 
 def save_as(nb):
@@ -24,14 +23,39 @@ def set_notebook_name(nb, name):
 
 def test_save_readonly_as(notebook_frontend):
     print('[Test] [test_save_readonly_as]')
-    notebook_frontend.edit_cell(index=0, content='a=10; print(a)')
     notebook_frontend.wait_for_kernel_ready()
-    notebook_frontend.wait_for_selector(".input", page=EDITOR_PAGE)
 
-    # Set a name for comparison later
-    print('[Test] Set notebook name')
-    set_notebook_name(notebook_frontend, name="nb1.ipynb")
-    assert get_notebook_name(notebook_frontend) == "nb1.ipynb"
+    print('[Test] Make notebook read-only')
+    cell_text = (
+        'import os\nimport stat\nos.chmod("'
+        + notebook_frontend.get_page_url(EDITOR_PAGE).split('?')[0].split('/')[-1]
+        + '", stat.S_IREAD)\nprint(0)'
+    )
+    notebook_frontend.edit_cell(index=0, content=cell_text)
+    notebook_frontend.wait_for_condition(
+        lambda: notebook_frontend.get_cell_contents(0).strip() == cell_text
+    )
+    notebook_frontend.evaluate("Jupyter.notebook.get_cell(0).execute();", page=EDITOR_PAGE)
+    notebook_frontend.wait_for_cell_output(0)
+    notebook_frontend.reload(EDITOR_PAGE)
+    notebook_frontend.wait_for_kernel_ready()
+
+    print('[Test] Check that the notebook is read-only')
+    notebook_frontend.wait_for_condition(
+        lambda: notebook_frontend.evaluate('() => { return Jupyter.notebook.writable }', page=EDITOR_PAGE) is False,
+        timeout=150,
+        period=1
+    )
+
+    # Add some content
+    print('[Test] Add cell')
+    test_content_0 = "print('a simple')\nprint('test script')"
+    notebook_frontend.edit_cell(index=0, content=test_content_0)
+    notebook_frontend.wait_for_condition(
+        lambda: notebook_frontend.get_cell_contents(0).strip() == test_content_0,
+        timeout=150,
+        period=1
+    )
 
     # Wait for Save As modal, save
     print('[Test] Save')
@@ -57,7 +81,7 @@ def attempt_form_fill_and_save():
         # ....................
         # This may be a retry, check if the application state reflects a successful save operation
         nonlocal fill_attempts
-        if fill_attempts and get_notebook_name(notebook_frontend) == "new_notebook.ipynb":
+        if fill_attempts and get_notebook_name(notebook_frontend) == "writable_notebook.ipynb":
             print('[Test]   Success from previous save attempt!')
             return True
         fill_attempts += 1
@@ -70,18 +94,18 @@ def attempt_form_fill_and_save():
 
         # Set the notebook name field in the save dialog
         print('[Test] Fill the input field')
-        name_input_element.evaluate(f'(elem) => {{ elem.value = "new_notebook.ipynb"; return elem.value; }}')
+        name_input_element.evaluate(f'(elem) => {{ elem.value = "writable_notebook.ipynb"; return elem.value; }}')
         notebook_frontend.wait_for_condition(
             lambda: name_input_element.evaluate(
-                f'(elem) => {{ elem.value = "new_notebook.ipynb"; return elem.value; }}') == 'new_notebook.ipynb',
+                f'(elem) => {{ elem.value = "writable_notebook.ipynb"; return elem.value; }}') == 'writable_notebook.ipynb',
             timeout=120,
             period=.25
         )
         # Show the input field value
         print('[Test] Name input field contents:')
         field_value = name_input_element.evaluate(f'(elem) => {{ return elem.value; }}')
         print('[Test]   ' + field_value)
-        if field_value != 'new_notebook.ipynb':
+        if field_value != 'writable_notebook.ipynb':
             return False
 
         print('[Test] Locate and click the save button')
@@ -96,13 +120,13 @@ def attempt_form_fill_and_save():
             print('[Test] Save element still visible after save, wait for hidden')
             try:
                 save_element.expect_not_to_be_visible(timeout=120)
-            except PlaywrightTimeoutError as err:
+            except EndToEndTimeout as err:
                 traceback.print_exc()
                 print('[Test]   Save button failed to hide...')
 
         # Check if the save operation succeeded (by checking notebook name change)
         notebook_frontend.wait_for_condition(
-            lambda: get_notebook_name(notebook_frontend) == "new_notebook.ipynb", timeout=120, period=5
+            lambda: get_notebook_name(notebook_frontend) == "writable_notebook.ipynb", timeout=120, period=5
         )
         print(f'[Test] Notebook name: {get_notebook_name(notebook_frontend)}')
         print('[Test] Notebook name was changed!')
@@ -113,4 +137,41 @@ def attempt_form_fill_and_save():
 
     # Test that address bar was updated
     print('[Test] Test address bar')
-    assert "new_notebook.ipynb" in notebook_frontend.get_page_url(page=EDITOR_PAGE)
+    assert "writable_notebook.ipynb" in notebook_frontend.get_page_url(page=EDITOR_PAGE)
+
+    print('[Test] Check that the notebook is no longer read only')
+    notebook_frontend.wait_for_condition(
+        lambda: notebook_frontend.evaluate('() => { return Jupyter.notebook.writable }', page=EDITOR_PAGE) is True,
+        timeout=150,
+        period=1
+    )
+
+    print('[Test] Add some more content')
+    test_content_1 = "print('a second simple')\nprint('script to test save feature')"
+    notebook_frontend.add_and_execute_cell(content=test_content_1)
+    # and save the notebook
+    notebook_frontend.evaluate("Jupyter.notebook.save_notebook()", page=EDITOR_PAGE)
+
+    print('[Test] Test that it still contains the content')
+    notebook_frontend.wait_for_condition(
+        lambda: notebook_frontend.get_cell_contents(index=0) == test_content_0,
+        timeout=150,
+        period=1
+    )
+    notebook_frontend.wait_for_condition(
+        lambda: notebook_frontend.get_cell_contents(index=1) == test_content_1,
+        timeout=150,
+        period=1
+    )
+    print('[Test] Test that it persists even after a refresh')
+    notebook_frontend.reload(EDITOR_PAGE)
+    notebook_frontend.wait_for_condition(
+        lambda: notebook_frontend.get_cell_contents(index=0) == test_content_0,
+        timeout=150,
+        period=1
+    )
+    notebook_frontend.wait_for_condition(
+        lambda: notebook_frontend.get_cell_contents(index=1) == test_content_1,
+        timeout=150,
+        period=1
+    )

nbclassic/tests/end_to_end/utils.py

@@ -33,7 +33,7 @@
 CELL_OUTPUT_SELECTOR = '.output_subarea'
 
 
-class TimeoutError(Exception):
+class EndToEndTimeout(Exception):
 
     def get_result(self):
         return None if not self.args else self.args[0]
@@ -179,6 +179,8 @@ def expect_not_to_be_visible(self, timeout=30):
             expect(self._element).not_to_be_visible(timeout=timeout * seconds_to_milliseconds)
         except ValueError as err:
             raise Exception('Cannot expect not_to_be_visible on this type!') from err
+        except AssertionError as err:
+            raise EndToEndTimeout('Error waiting not_to_be_visible!') from err
 
     def expect_to_have_text(self, text):
         try:
@@ -393,6 +395,17 @@ def evaluate(self, text, page):
     def _pause(self):
         self._editor_page.pause()
 
+    def reload(self, page):
+        """Find an element matching selector on the given page"""
+        if page == TREE_PAGE:
+            specified_page = self._tree_page
+        elif page == EDITOR_PAGE:
+            specified_page = self._editor_page
+        else:
+            raise Exception('Error, provide a valid page to locate from!')
+
+        specified_page.reload()
+
     def locate(self, selector, page):
         """Find an element matching selector on the given page"""
         if page == TREE_PAGE:
@@ -662,7 +675,7 @@ def wait_for_condition(self, check_func, timeout=30, period=.1):
                 traceback.print_exc()
                 print('\n[NotebookFrontend] Ignoring exception in wait_for_condition, read more above')
         else:
-            raise TimeoutError()
+            raise EndToEndTimeout()
 
     def wait_for_cell_output(self, index=0, timeout=30):
         """Waits for the cell to finish executing and return the cell output"""