Merge pull request #146 from krassowski/tests/feature/completion

Add initial tests for completion feature

Author: krassowski

CHANGELOG.md

@@ -16,8 +16,8 @@
     [#129](https://github.com/krassowski/jupyterlab-lsp/pull/129)
     )
   - added a widget panel with diagnostics (inspections), allowing to
-    sort and explore diagnostics, and to go-to the respective location
-    in code (on click); accessible from the context menu (
+    sort and explore diagnostics, and to go to the respective location
+    in code (with a click); accessible from the context menu (
     [#127](https://github.com/krassowski/jupyterlab-lsp/pull/127)
     )
   - all commands are now accessible from the command palette (
@@ -37,6 +37,9 @@
   - fixed LSP of R in Python (`%%R` magic cell from rpy2) (
     [#144](https://github.com/krassowski/jupyterlab-lsp/pull/144)
     )
+  - completion now work properly when the kernel is shut down (
+    [#146](https://github.com/krassowski/jupyterlab-lsp/pull/146)
+    )
 
 ## `lsp-ws-connection 0.3.0`
 

atest/05_Features/Completion.robot

@@ -0,0 +1,79 @@
+*** Settings ***
+Suite Setup       Setup Suite For Screenshots    completion
+Resource          ../Keywords.robot
+
+*** Variables ***
+${STATUSBAR}      css:div.lsp-statusbar-item
+${COMPLETER_BOX}    css:.jp-Completer.jp-HoverBox
+
+*** Test Cases ***
+Works With Kernel Running
+    [Documentation]    The suggestions from kernel and LSP should get integrated.
+    Setup Notebook    Python    Completion.ipynb
+    Wait Until Element Contains    ${STATUSBAR}    Fully initialized    timeout=20s
+    Enter Cell Editor    1    line=2
+    Capture Page Screenshot    01-entered-cell.png
+    Trigger Completer
+    Capture Page Screenshot    02-completions-shown.png
+    # lowercase and uppercase suggestions:
+    Completer Should Suggest    TabError
+    # this comes from LSP:
+    Completer Should Suggest    test
+    # this comes from kernel
+    Completer Should Suggest    %%timeit
+    Press Keys    None    ENTER
+    Capture Page Screenshot    03-completion-confirmed.png
+    ${content} =    Get Cell Editor Content    1
+    Should Contain    ${content}    TabError
+    [Teardown]    Clean Up After Working With File    Completion.ipynb
+
+Works When Kernel Is Shut Down
+    Setup Notebook    Python    Completion.ipynb
+    Lab Command    Shut Down All Kernels…
+    Capture Page Screenshot    01-shutting-kernels.png
+    Accept Default Dialog Option
+    Capture Page Screenshot    02-kernels-shut.png
+    Enter Cell Editor    1    line=2
+    Trigger Completer
+    Capture Page Screenshot    03-completions-shown.png
+    # this comes from LSP:
+    Completer Should Suggest    test
+    # this comes from kernel:
+    Completer Should Not Suggest    %%timeit
+    [Teardown]    Clean Up After Working With File    Completion.ipynb
+
+Autocompletes If Only One Option
+    Setup Notebook    Python    Completion.ipynb
+    Enter Cell Editor    3    line=1
+    Press Keys    None    cle
+    Press Keys    None    TAB
+    Wait Until Keyword Succeeds    20x    0.5s    Cell Editor Should Equal    3    list.clear
+    [Teardown]    Clean Up After Working With File    Completion.ipynb
+
+*** Keywords ***
+Enter Cell Editor
+    [Arguments]    ${cell_nr}    ${line}=1
+    Click Element    css:.jp-CodeCell:nth-child(${cell_nr}) .CodeMirror-line:nth-child(${line})
+    Wait Until Page Contains Element    css:.jp-CodeCell:nth-child(${cell_nr}) .CodeMirror-focused
+
+Get Cell Editor Content
+    [Arguments]    ${cell_nr}
+    ${content}    Execute JavaScript    return document.querySelector('.jp-CodeCell:nth-child(${cell_nr}) .CodeMirror').CodeMirror.getValue()
+    [Return]    ${content}
+
+Cell Editor Should Equal
+    [Arguments]    ${cell}    ${value}
+    ${content} =    Get Cell Editor Content    ${cell}
+    Should Be Equal    ${content}    ${value}
+
+Completer Should Suggest
+    [Arguments]    ${text}
+    Page Should Contain Element    ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]
+
+Completer Should Not Suggest
+    [Arguments]    ${text}
+    Page Should Not Contain Element    ${COMPLETER_BOX} .jp-Completer-item[data-value="${text}"]
+
+Trigger Completer
+    Press Keys    None    TAB
+    Wait Until Page Contains Element    ${COMPLETER_BOX}    timeout=6s

atest/examples/Completion.ipynb

@@ -0,0 +1,53 @@
+{
+ "cells": [
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "test = 1 \n",
+    "t"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "list"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "list."
+   ]
+  }
+ ],
+ "metadata": {
+  "kernelspec": {
+   "display_name": "Python 3",
+   "language": "python",
+   "name": "python3"
+  },
+  "language_info": {
+   "codemirror_mode": {
+    "name": "ipython",
+    "version": 3
+   },
+   "file_extension": ".py",
+   "mimetype": "text/x-python",
+   "name": "python",
+   "nbconvert_exporter": "python",
+   "pygments_lexer": "ipython3",
+   "version": "3.8.0"
+  }
+ },
+ "nbformat": 4,
+ "nbformat_minor": 4
+}

packages/jupyterlab-lsp/src/adapters/jupyterlab/components/completion.ts

@@ -67,6 +67,10 @@ export class LSPConnector extends DataConnector<
     this.options = options;
   }
 
+  protected get _has_kernel(): boolean {
+    return this.options.session.kernel !== null;
+  }
+
   protected get _kernel_language(): string {
     return this.options.session.kernel.info.language_info.name;
   }
@@ -127,6 +131,7 @@ export class LSPConnector extends DataConnector<
 
     try {
       if (
+        this._has_kernel &&
         this._kernel_connector &&
         // TODO: this would be awesome if we could connect to rpy2 for R suggestions in Python,
         //  but this is not the job of this extension; nevertheless its better to keep this in
@@ -157,10 +162,11 @@ export class LSPConnector extends DataConnector<
         virtual_cursor,
         document
       ).catch(e => {
-        console.log(e);
+        console.warn('LSP: hint failed', e);
         return this.fallback_connector.fetch(request);
       });
     } catch (e) {
+      console.warn('LSP: kernel completions failed', e);
       return this.fallback_connector.fetch(request);
     }
   }

packages/jupyterlab-lsp/src/adapters/jupyterlab/jl_adapter.ts

@@ -210,7 +210,7 @@ export abstract class JupyterLabWidgetAdapter
 
     await this.virtual_editor.update_documents().then(() => {
       // refresh the document on the LSP server
-      this.document_changed(virtual_document);
+      this.document_changed(virtual_document, true);
       console.log(
         'LSP: virtual document(s) for',
         this.document_path,
@@ -229,7 +229,7 @@ export abstract class JupyterLabWidgetAdapter
     });
   }
 
-  document_changed(virtual_document: VirtualDocument) {
+  document_changed(virtual_document: VirtualDocument, is_init = false) {
     // TODO only send the difference, using connection.sendSelectiveChange()
     let connection = this.connection_manager.connections.get(
       virtual_document.id_path
@@ -251,18 +251,22 @@ export abstract class JupyterLabWidgetAdapter
       'has changed sending update'
     );
     connection.sendFullTextChange(virtual_document.value);
-    // guarantee that the virtual editor won't perform an update of the virtual documents while
-    // the changes are recorded...
-    // TODO this is not ideal - why it solves the problem of some errors,
-    //  it introduces an unnecessary delay. A better way could be to invalidate some of the updates when a new one comes in.
-    //  but maybe not every one (then the outdated state could be kept for too long fo a user who writes very quickly)
-    //  also we would not want to invalidate the updates for the purpose of autocompletion (the trigger characters)
-    this.virtual_editor
-      .with_update_lock(async () => {
-        await adapter.updateAfterChange();
-      })
-      .then()
-      .catch(console.warn);
+    // the first change (initial) is not propagated to features,
+    // as it has no associated CodeMirrorChange object
+    if (!is_init) {
+      // guarantee that the virtual editor won't perform an update of the virtual documents while
+      // the changes are recorded...
+      // TODO this is not ideal - why it solves the problem of some errors,
+      //  it introduces an unnecessary delay. A better way could be to invalidate some of the updates when a new one comes in.
+      //  but maybe not every one (then the outdated state could be kept for too long fo a user who writes very quickly)
+      //  also we would not want to invalidate the updates for the purpose of autocompletion (the trigger characters)
+      this.virtual_editor
+        .with_update_lock(async () => {
+          await adapter.updateAfterChange();
+        })
+        .then()
+        .catch(console.warn);
+    }
   }
 
   private async connect_adapter(

packages/jupyterlab-lsp/src/adapters/jupyterlab/notebook.ts

@@ -47,6 +47,10 @@ export class NotebookAdapter extends JupyterLabWidgetAdapter {
       .catch(console.warn);
 
     this.widget.context.session.kernelChanged.connect((_session, change) => {
+      if (!change.newValue) {
+        console.warn('LSP: kernel was shut down');
+        return;
+      }
       change.newValue.ready
         .then(async spec => {
           console.log(

packages/jupyterlab-lsp/src/virtual/editor.ts

@@ -102,7 +102,7 @@ export abstract class VirtualEditor implements CodeMirror.Editor {
    * @param fn - the callback to execute in update lock
    */
   public async with_update_lock(fn: Function) {
-    await until_ready(() => this.can_update(), 10, 10).then(() => {
+    await until_ready(() => this.can_update(), 12, 10).then(() => {
       try {
         this.update_lock = true;
         fn();