Repository: jupyterhub/jupyter-server-proxy (language: python)

Issue:
Tests broken for jupyter_server 2 - help wanted!
Our jupyterlab extension tests for JupyterLab 2 and JupyterLab 3 are both failing when jupyter_server 2 is used, which it is in our Python 3.11 tests but not in our Python 3.7 tests because jupyter_server 2 requires python 3.8 and is not installed in the python 3.7 context.

# Summary to help understand the error

I've debugged this in a fork where I run tests in a Python 3.8 environment, and only switch between installing jupyter_server 1 and 2. Doing so, I conclude the following two separate errors we encounter when testing with jupyterlab 2 and 3 together with jupyter_server 2.

## JupyterLab 2 failure

When using jupyterlab 2 and jupyter_server 2 in a Python 3.8 environment, we fail in the `python -m jupyterlab.browser_check` step, and here are some diff from logs and python dependencies installed.

```diff
-# lab 2, py3.8, jupyter_server 1, notebook
+# lab 2, py3.8, jupyter_server 2, notebook
    app dir: /opt/hostedtoolcache/Python/3.8.15/x64/share/jupyter/lab
         @jupyterhub/jupyter-server-proxy v3.2.2  enabled  OK*
 JupyterLab v2.3.2
@@ -83,11 +83,21 @@ Starting Chrome Headless
 Navigating to page: file:///home/runner/.local/share/jupyter/runtime/nbserver-x-open.html
 Waiting for page to load...
 Waiting for application to start...
+/opt/hostedtoolcache/Python/3.8.15/x64/lib/python3.8/site-packages/jupyter_server/base/handlers.py:240: UserWarning: The Tornado web application does not have an 'identity_provider' defined in its settings. In future releases of jupyter_server, this will be a required key for all subclasses of `JupyterHandler`. For an example, see the jupyter_server source code for how to add an identity provider to the tornado settings: https://github.com/jupyter-server/jupyter_server/blob/aa8fd8b3faf37466eeb99689d5555314c5bf6640/jupyter_server/serverapp.py#L253
+  warnings.warn(
+[E x BrowserApp] Error unpacking user from cookie: Extra data: line 1 column 2 (char 1)
+Error unpacking user from cookie: Extra data: line 1 column 2 (char 1)
+[W x BrowserApp] Clearing invalid/expired login cookie username-127-0-0-1-8889
+[I x BrowserApp] 302 GET /foo/server-proxy/servers-info (127.0.0.1) 5.060000ms
+Parsed an error from text content: undefined Plugin '@jupyterhub/jupyter-server-proxy:main' failed to activate.
+Parsed an error from text content: undefined SyntaxError: Unexpected token < in JSON at position 0
 Chrome test complete
 [I x BrowserApp] Test Complete
-[I x BrowserApp] Exiting normally
+[C x BrowserApp] Exiting with 1 due to errors
+Exiting with 1 due to errors
 [I x BrowserApp] Stopping server...
 [W x BrowserApp] Failed to fetch package metadata for '@jupyterhub/jupyter-server-proxy': <HTTPError 404: 'Not Found'>
+Error: Process completed with exit code 1.
 
-jupyter-server==1.23.4
+jupyter_server==2.0.6
```

## JupyterLab 3 failure

When using jupyterlab 3 and jupyter_server 2 in a Python 3.8 environment, we fail in the "browser acceptance test" section, and pass the `python -m jupyterlab.browser_check` step that we fail with when using jupyterlab 2.

```diff
-# jupyter_server 1, lab 3, py3.8, notebook
+# jupyter_server 2, lab 3, py3.8, notebook
 ============================= test session starts ==============================
 platform linux -- Python 3.8.15, pytest-7.2.0, pluggy-1.0.0
 rootdir: /home/runner/work/jupyter-server-proxy/jupyter-server-proxy
@@ -12,32 +12,62 @@ Acceptance.Classic :: Server Proxies in Notebook Classic
 ==============================================================================
 Notebook Classic Loads                                                | PASS |
 ------------------------------------------------------------------------------
-Launch Browser Tab                                                    | PASS |
+Launch Browser Tab                                                    | FAIL |
+Element with locator 'css:a[role="menuitem"][href*="foo"]' not found.
 ------------------------------------------------------------------------------
-Launch Another Browser Tab                                            | PASS |
+Launch Another Browser Tab                                            | FAIL |
+Element with locator 'css:a[role="menuitem"][href*="bar"]' not found.
 ------------------------------------------------------------------------------
-Acceptance.Classic :: Server Proxies in Notebook Classic              | PASS |
-3 tests, 3 passed, 0 failed
+Acceptance.Classic :: Server Proxies in Notebook Classic              | FAIL |
+3 tests, 1 passed, 2 failed
 ==============================================================================
 Acceptance.Lab :: Server Proxies in Lab                                       
 ==============================================================================
-Lab Loads                                                             | PASS |
+Lab Loads                                                             | FAIL |
+Parent suite setup failed:
+Element 'xpath://script[contains(@id, 'jupyter-config-data')]' did not appear in 5 seconds.
 ------------------------------------------------------------------------------
-Launch Browser Tab                                                    | PASS |
+Launch Browser Tab                                                    | FAIL |
+Parent suite setup failed:
+Element 'xpath://script[contains(@id, 'jupyter-config-data')]' did not appear in 5 seconds.
 ------------------------------------------------------------------------------
-Launch Lab Tab                                                        | PASS |
+Launch Lab Tab                                                        | FAIL |
+Parent suite setup failed:
+Element 'xpath://script[contains(@id, 'jupyter-config-data')]' did not appear in 5 seconds.
 ------------------------------------------------------------------------------
-Acceptance.Lab :: Server Proxies in Lab                               | PASS |
-3 tests, 3 passed, 0 failed
+Acceptance.Lab :: Server Proxies in Lab                               | FAIL |
+Suite setup failed:
+Element 'xpath://script[contains(@id, 'jupyter-config-data')]' did not appear in 5 seconds.
+
+3 tests, 0 passed, 3 failed
 ==============================================================================
-Acceptance :: Acceptance tests for jupyter-server-proxy               | PASS |
-6 tests, 6 passed, 0 failed
+Acceptance :: Acceptance tests for jupyter-server-proxy               | FAIL |
+6 tests, 1 passed, 5 failed
 ==============================================================================
 Output:  /home/runner/work/jupyter-server-proxy/jupyter-server-proxy/build/robot/output.xml
 Log:     /home/runner/work/jupyter-server-proxy/jupyter-server-proxy/build/robot/log.html
 Report:  /home/runner/work/jupyter-server-proxy/jupyter-server-proxy/build/robot/report.html
-.
+F
+
+=================================== FAILURES ===================================
+__________________________________ test_robot __________________________________
+
+    def test_robot():
+        """run acceptance tests with robotframework"""
+        with_robot = pytest.importorskip("JupyterLibrary")
+    
+        if OUTPUT.exists():
+            shutil.rmtree(OUTPUT)
+    
+        return_code = subprocess.call(
+            ["robot", "--consolecolors=on", f"--outputdir={OUTPUT}", str(HERE)],
+            cwd=str(HERE)
+        )
+    
+>       assert return_code == 0
+E       assert 5 == 0
 
+tests/acceptance/test_acceptance.py:22: AssertionError
 =============================== warnings summary ===============================
 ../../../../../opt/hostedtoolcache/Python/3.8.15/x64/lib/python3.8/site-packages/jupyter_server/base/handlers.py:x
   /opt/hostedtoolcache/Python/3.8.15/x64/lib/python3.8/site-packages/jupyter_server/base/handlers.py:x: DeprecationWarning: Jupyter is migrating its paths to use standard platformdirs
@@ -48,7 +78,9 @@ Report:  /home/runner/work/jupyter-server-proxy/jupyter-server-proxy/build/robot
     from jupyter_core.paths import is_hidden
 
 -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
-================= 1 passed, 34 deselected, 1 warning in 27.61s =================
+=========================== short test summary info ============================
+FAILED tests/acceptance/test_acceptance.py::test_robot - assert 5 == 0
+================= 1 failed, 34 deselected, 1 warning in 43.14s =================
 
-jupyter-server==1.23.4
+jupyter_server==2.0.6
```


Discussion / hints:
Note that next to that text log of the robot test there are some HTML logs that might provide some more content (e.g. screenshots).

There are also some ways to embed the robot report directly into a `pytest-html` report, but it doesn't do as well with liked assets (like screenshots) without some care and feeding.

> `python -m jupyterlab.browser_check`

If you are basing some CI test on this on `ubuntu-latest`, it's been known not to work, with [this fix on ipywidgets](https://github.com/jupyter-widgets/ipywidgets/pull/3653/files#diff-1db27d93186e46d3b441ece35801b244db8ee144ff1405ca27a163bfe878957fR39).

Another approach is to move the various browser deps to a conda-managed environment, where one can specify a known-good browser/webdriver pair. 

> I've debugged this in a fork

Can you open this as a (draft) PR so the full logs get uploaded? 
@bollwyvl thank you for chiming in!!

The test logs are available at https://github.com/consideRatio/jupyter-server-proxy/actions/runs/3800291613. It is a fork branch started out as a branch to resolve #363, but I ended up debugging this test failure.

I see that some reports have been uploaded as seen here: https://github.com/consideRatio/jupyter-server-proxy/actions/runs/3800291613. Specifically, I note that the artifact [`acceptance-tests--lab-3-55`](https://github.com/consideRatio/jupyter-server-proxy/suites/10098393114/artifacts/492313712) is related to [this test failure](https://github.com/consideRatio/jupyter-server-proxy/actions/runs/3800291613/jobs/6463627999). I conclude that also the equivalent test with jupyter_server 1 was uploading the artifact under the same name... I'll work that to see a difference! Thanks for your guidance @bollwyvl!!

The tests have been run in ubuntu-20.04:

```yaml
  test:
    # FIXME: We need ubutnu-20.04 until firefox and geckowebdriver is available
    #        in ubuntu-22.04. To check if we can upgrade to ubuntu-22.04, see
    #        https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md#browsers-and-drivers
    #
    runs-on: ubuntu-20.04
```

In https://github.com/consideRatio/jupyter-server-proxy/actions/runs/3801386558 we now have proper artifacts for jupyter_server 1 vs jupyter_server 2.

When using JupyterLab 3 and we pass the jupyterlab.browser_check as we only do for JupyterLab 3 and not 2, we can see the following shows up.

![image](https://user-images.githubusercontent.com/3837114/209972953-041f74ad-4828-4796-ba45-87a522654411.png)

This seems very related to the jupyterlab.browser_check failure observed when using JupyterLab 2, with logs about:

```
Waiting for application to start...
/opt/hostedtoolcache/Python/3.8.15/x64/lib/python3.8/site-packages/jupyter_server/base/handlers.py:240: UserWarning: The Tornado web application does not have an 'identity_provider' defined in its settings. In future releases of jupyter_server, this will be a required key for all subclasses of `JupyterHandler`. For an example, see the jupyter_server source code for how to add an identity provider to the tornado settings: https://github.com/jupyter-server/jupyter_server/blob/aa8fd8b3faf37466eeb99689d5555314c5bf6640/jupyter_server/serverapp.py#L253
  warnings.warn(
[E x BrowserApp] Error unpacking user from cookie: Extra data: line 1 column 2 (char 1)
Error unpacking user from cookie: Extra data: line 1 column 2 (char 1)
[W x BrowserApp] Clearing invalid/expired login cookie username-127-0-0-1-8889
[I x BrowserApp] 302 GET /foo/server-proxy/servers-info (127.0.0.1) 5.060000ms
Parsed an error from text content: undefined Plugin '@jupyterhub/jupyter-server-proxy:main' failed to activate.
Parsed an error from text content: undefined SyntaxError: Unexpected token < in JSON at position 0
```
Welp, that page definitely isn't JupyterLab, and also doesn't contain a `jupyter-config-data` for it to [find here](https://github.com/jupyterlab/jupyterlab/blob/edc294b06fe6607be80094dd590cb9f766ab83ff/jupyterlab/browser-test.js#L96).

So it could be the newer server is louder about configuration errors, which will likely need #363 to get past.
@bollwyvl the page comes from jupyter_server complaining about auth, which probably relates to the messages about identity_provider and/or error about unpacking user from cookie.

I don't think this is related to #363 btw, but I'm not sure at all. This is tricky to debug =/

## Related

- BrowserApp definition: https://github.com/jupyterlab/jupyterlab/blob/edc294b06fe6607be80094dd590cb9f766ab83ff/jupyterlab/browser_check.py#L165
@bollwyvl btw I think firefox and geckodriver is available in ubuntu-22.04 runners now, see https://github.com/actions/runner-images/commit/ac8ae6b0eeeec50305bf54ffdf588d5bbe9fc145#diff-d3f496e99457b699df2a5621c8f59cb52f60a050e1a88318af7d6f7069c12b2eR164-R180 that was merged ~a month ago.
- **jupyter_server 2+ should only be tested against jupyterlab 3.5+**
  We had an error with jupyter_server 2 + jupyterlab 2, but that wasn't meant to work as jupyter_server 2 is only supported by jupyterlab 3.5+. See https://github.com/conda-forge/jupyterlab-feedstock/commit/38a84312746c59ca39ed8f93644c008d00fdd9e6.
- I opened https://github.com/robots-from-jupyter/robotframework-jupyterlibrary/issues/65 where I hope we can conclude if the issue is in this repo or upstream in the robots-from-jupyter/robotframework-jupyterlibrary repo by running that projects test suite

Fix patch:
diff --git a/setup.py b/setup.py
index a12f910d..1613a741 100644
--- a/setup.py
+++ b/setup.py
@@ -97,7 +97,7 @@
             "pytest-html"
         ],
         "acceptance": [
-            "robotframework-jupyterlibrary"
+            "robotframework-jupyterlibrary>=0.4.2"
         ]
     },
     zip_safe=False,


Test patch:
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
index 5ab048a2..92ea3a5d 100644
--- a/.github/workflows/test.yaml
+++ b/.github/workflows/test.yaml
@@ -79,7 +79,7 @@ jobs:
         if: always()
         uses: actions/upload-artifact@v3
         with:
-          name: unit-tests-${{matrix.python }}-${{ matrix.jupyter-app }}-${{matrix.jupyterlab-version}}-${{ github.run_number }}
+          name: unit-tests-${{ matrix.python-version }}-${{ matrix.jupyter-app }}-${{ matrix.jupyterlab-version }}-${{ github.run_number }}
           path: |
             ./build/pytest
             ./build/coverage
@@ -103,6 +103,12 @@ jobs:
           jupyter lab build --minimize=False --debug
 
       - name: Check the lab extension
+        # We test the labextension thoroughly in the acceptance tests below, so
+        # we have conditionally disabled this basic check is to avoid issues in
+        # jupyterlab.browser_check with jupyterlab 2 and a modern version of
+        # python (3.11+).
+        #
+        if: ${{ !(matrix.jupyterlab-version == '2' && startsWith(matrix.python-version, '3.1')) }}
         run: |
           jupyter labextension list
           jupyter labextension list 2>&1 | grep -ie '@jupyterlab/server-proxy.*OK.*'
@@ -111,7 +117,7 @@ jobs:
       - name: Install Acceptance test dependencies
         run: |
           # the acceptance test requires notebook to run
-          pip install "notebook<7" robotframework-jupyterlibrary
+          pip install "notebook<7" "robotframework-jupyterlibrary>=0.4.2"
 
       - name: Run acceptance tests
         run: |
@@ -121,6 +127,6 @@ jobs:
         if: always()
         uses: actions/upload-artifact@v3
         with:
-          name: acceptance-tests-${{ matrix.python }}-${{ matrix.jupyter-app }}-${{ matrix.jupyterlab-version }}-${{ github.run_number }}
+          name: acceptance-tests-${{ matrix.python-version }}-${{ matrix.jupyter-app }}-${{ matrix.jupyterlab-version }}-${{ github.run_number }}
           path: |
             ./build/robot
diff --git a/tests/acceptance/__init__.robot b/tests/acceptance/__init__.robot
index 99d6361f..f774959e 100644
--- a/tests/acceptance/__init__.robot
+++ b/tests/acceptance/__init__.robot
@@ -13,10 +13,12 @@ Suite Teardown    Clean Up
 Set Up
     ${notebook dir} =    Set Variable    ${OUTPUT DIR}${/}notebooks
     Copy Directory    resources    ${notebook dir}
+    Set Screenshot Directory   EMBED
     Set Environment Variable
     ...    name=JUPYTER_CONFIG_DIR
     ...    value=${notebook dir}
     Wait For New Jupyter Server To Be Ready
+    ...    %{JUPYTER_LIBRARY_APP_COMMAND}
     ...    stdout=${OUTPUT DIR}${/}server.log
     ...    notebook_dir=${notebook dir}
     ...    cwd=${notebook dir}
diff --git a/tests/acceptance/resources/jupyter_notebook_config.json b/tests/acceptance/resources/jupyter_config.json
similarity index 100%
rename from tests/acceptance/resources/jupyter_notebook_config.json
rename to tests/acceptance/resources/jupyter_config.json
diff --git a/tests/acceptance/test_acceptance.py b/tests/acceptance/test_acceptance.py
index 785ab5ad..187b6fe0 100644
--- a/tests/acceptance/test_acceptance.py
+++ b/tests/acceptance/test_acceptance.py
@@ -1,22 +1,48 @@
-import subprocess
-import pytest
 from pathlib import Path
+import os
 import shutil
+import subprocess
+
+import pytest
 
 HERE = Path(__file__).parent
 OUTPUT = HERE.parent.parent / "build/robot"
+JUPYTER_SERVER_INFO = None
+
+try:
+    import jupyter_server
+    JUPYTER_SERVER_INFO = jupyter_server.version_info
+except ImportError:
+    pass
 
 
 def test_robot():
     """run acceptance tests with robotframework"""
     with_robot = pytest.importorskip("JupyterLibrary")
 
+    env = dict(**os.environ)
+
+    # JUPYTER_LIBRARY_* env vars documentation:
+    # https://robotframework-jupyterlibrary.readthedocs.io/en/stable/LIMITS.html#notebookapp-vs-serverapp
+    if JUPYTER_SERVER_INFO is None:
+        env.update(
+            JUPYTER_LIBRARY_APP_COMMAND="jupyter-notebook",
+            JUPYTER_LIBRARY_APP="NotebookApp",
+        )
+    else:
+        env.update(
+            JUPYTER_LIBRARY_APP_COMMAND="jupyter-server",
+            JUPYTER_LIBRARY_APP="ServerApp",
+        )
+
+
     if OUTPUT.exists():
         shutil.rmtree(OUTPUT)
 
     return_code = subprocess.call(
         ["robot", "--consolecolors=on", f"--outputdir={OUTPUT}", str(HERE)],
-        cwd=str(HERE)
+        cwd=str(HERE),
+        env=env
     )
 
     assert return_code == 0