Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions .github/workflows/enforce-label.yml

This file was deleted.

9 changes: 0 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# jupyterlab_deepnote

[![Github Actions Status](https://github.com/deepnote/jupyterlab-deepnote/workflows/Build/badge.svg)](https://github.com/deepnote/jupyterlab-deepnote/actions/workflows/build.yml)

A Deepnote extension for JupyterLab

This extension is composed of a Python package named `jupyterlab_deepnote`
Expand Down Expand Up @@ -168,13 +166,6 @@ jlpm
jlpm test
```

#### Integration tests

This extension uses [Playwright](https://playwright.dev/docs/intro) for the integration tests (aka user level tests).
More precisely, the JupyterLab helper [Galata](https://github.com/jupyterlab/jupyterlab/tree/master/galata) is used to handle testing the extension in JupyterLab.

More information are provided within the [ui-tests](./ui-tests/README.md) README.

### Packaging the extension

See [RELEASE](RELEASE.md)
7 changes: 7 additions & 0 deletions src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,18 @@ class NotebookPicker extends ReactWidget {

return (
<HTMLSelect
id="deepnote-notebook-picker"
value={this.selected ?? '-'}
onChange={this.handleChange}
onKeyDown={() => {}}
aria-label="Select active notebook"
title="Select active notebook"
style={{
maxWidth: '120px',
textOverflow: 'ellipsis',
whiteSpace: 'nowrap',
overflow: 'hidden'
}}
>
{names.length === 0 ? (
<option value="-">-</option>
Expand Down
167 changes: 167 additions & 0 deletions ui-tests/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
# Integration Testing

This folder contains the integration tests of the extension.

They are defined using [Playwright](https://playwright.dev/docs/intro) test runner
and [Galata](https://github.com/jupyterlab/jupyterlab/tree/main/galata) helper.

The Playwright configuration is defined in [playwright.config.js](./playwright.config.js).

The JupyterLab server configuration to use for the integration test is defined
in [jupyter_server_test_config.py](./jupyter_server_test_config.py).

The default configuration will produce video for failing tests and an HTML report.

> There is a UI mode that you may like; see [that video](https://www.youtube.com/watch?v=jF0yA-JLQW0).
## Run the tests

> All commands are assumed to be executed from the root directory
To run the tests, you need to:

1. Compile the extension:

```sh
jlpm install
jlpm build:prod
```

> Check the extension is installed in JupyterLab.
2. Install test dependencies (needed only once):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Fix ordered-list numbering (MD029).

Use "1." for all items to satisfy markdownlint and keep auto-numbering consistent.

-2. Install test dependencies (needed only once):
+1. Install test dependencies (needed only once):
-3. Execute the [Playwright](https://playwright.dev/docs/intro) tests:
+1. Execute the [Playwright](https://playwright.dev/docs/intro) tests:
-2. Install test dependencies (needed only once):
+1. Install test dependencies (needed only once):
-3. Execute the [Playwright](https://playwright.dev/docs/intro) command:
+1. Execute the [Playwright](https://playwright.dev/docs/intro) command:
-2. Install test dependencies (needed only once):
+1. Install test dependencies (needed only once):
-3. Start the server:
+1. Start the server:
-4. Execute the [Playwright code generator](https://playwright.dev/docs/codegen) in **another terminal**:
+1. Execute the [Playwright code generator](https://playwright.dev/docs/codegen) in **another terminal**:
-2. Install test dependencies (needed only once):
+1. Install test dependencies (needed only once):
-3. Execute the Playwright tests in [debug mode](https://playwright.dev/docs/debug):
+1. Execute the Playwright tests in [debug mode](https://playwright.dev/docs/debug):

Also applies to: 41-41, 69-69, 78-78, 105-105, 114-114, 121-121, 143-143, 152-152

🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

32-32: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

🤖 Prompt for AI Agents
In ui-tests/README.md around lines 32 (and also apply same change at lines 41,
69, 78, 105, 114, 121, 143, 152), the ordered list uses explicit incremental
numbers which triggers MD029; change each ordered-list item’s leading number to
"1." so the markdown uses auto-numbering consistently and satisfies
markdownlint.


```sh
cd ./ui-tests
jlpm install
jlpm playwright install
cd ..
```

3. Execute the [Playwright](https://playwright.dev/docs/intro) tests:

```sh
cd ./ui-tests
jlpm playwright test
```

Test results will be shown in the terminal. In case of any test failures, the test report
will be opened in your browser at the end of the tests execution; see
[Playwright documentation](https://playwright.dev/docs/test-reporters#html-reporter)
for configuring that behavior.

## Update the tests snapshots

> All commands are assumed to be executed from the root directory
If you are comparing snapshots to validate your tests, you may need to update
the reference snapshots stored in the repository. To do that, you need to:

1. Compile the extension:

```sh
jlpm install
jlpm build:prod
```

> Check the extension is installed in JupyterLab.
2. Install test dependencies (needed only once):

```sh
cd ./ui-tests
jlpm install
jlpm playwright install
cd ..
```

3. Execute the [Playwright](https://playwright.dev/docs/intro) command:

```sh
cd ./ui-tests
jlpm playwright test -u
```

> Some discrepancy may occurs between the snapshots generated on your computer and
> the one generated on the CI. To ease updating the snapshots on a PR, you can
> type `please update playwright snapshots` to trigger the update by a bot on the CI.
> Once the bot has computed new snapshots, it will commit them to the PR branch.
Comment on lines +85 to +88
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Tighten wording/grammar.

Minor copy fixes.

-> Some discrepancy may occurs between the snapshots generated on your computer and
-> the one generated on the CI. To ease updating the snapshots on a PR, you can
+> Some discrepancies may occur between the snapshots generated on your computer and
+> the ones generated in CI. To ease updating snapshots on a PR, you can
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
> Some discrepancy may occurs between the snapshots generated on your computer and
> the one generated on the CI. To ease updating the snapshots on a PR, you can
> type `please update playwright snapshots` to trigger the update by a bot on the CI.
> Once the bot has computed new snapshots, it will commit them to the PR branch.
> Some discrepancies may occur between the snapshots generated on your computer and
> the ones generated in CI. To ease updating snapshots on a PR, you can
> type `please update playwright snapshots` to trigger the update by a bot on the CI.
> Once the bot has computed new snapshots, it will commit them to the PR branch.
🤖 Prompt for AI Agents
In ui-tests/README.md around lines 85 to 88, the wording and grammar in the
paragraph about snapshot discrepancies is awkward; rewrite it for clarity and
correctness. Replace "Some discrepancy may occurs between the snapshots
generated on your computer and the one generated on the CI." with a
grammatically correct sentence (e.g., "Snapshots generated locally may differ
from those produced on CI."), clarify that typing `please update playwright
snapshots` will trigger a bot to update snapshots on the CI, and make the final
sentence concise (e.g., "When the bot generates updated snapshots, it will
commit them to the PR branch."). Ensure subject-verb agreement, consistent
plurality, and concise phrasing.

## Create tests

> All commands are assumed to be executed from the root directory
To create tests, the easiest way is to use the code generator tool of playwright:

1. Compile the extension:

```sh
jlpm install
jlpm build:prod
```

> Check the extension is installed in JupyterLab.
2. Install test dependencies (needed only once):

```sh
cd ./ui-tests
jlpm install
jlpm playwright install
cd ..
```

3. Start the server:

```sh
cd ./ui-tests
jlpm start
```

4. Execute the [Playwright code generator](https://playwright.dev/docs/codegen) in **another terminal**:

```sh
cd ./ui-tests
jlpm playwright codegen localhost:8888
```

## Debug tests

> All commands are assumed to be executed from the root directory
To debug tests, a good way is to use the inspector tool of playwright:

1. Compile the extension:

```sh
jlpm install
jlpm build:prod
```

> Check the extension is installed in JupyterLab.
2. Install test dependencies (needed only once):

```sh
cd ./ui-tests
jlpm install
jlpm playwright install
cd ..
```

3. Execute the Playwright tests in [debug mode](https://playwright.dev/docs/debug):

```sh
cd ./ui-tests
jlpm playwright test --debug
```

## Upgrade Playwright and the browsers

To update the web browser versions, you must update the package `@playwright/test`:

```sh
cd ./ui-tests
jlpm up "@playwright/test"
jlpm playwright install
```
12 changes: 12 additions & 0 deletions ui-tests/jupyter_server_test_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
"""Server configuration for integration tests.
!! Never use this configuration in production because it
opens the server to the world and provide access to JupyterLab
JavaScript objects through the global window variable.
"""
Comment on lines +3 to +6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Docstring nits.

-!! Never use this configuration in production because it
-opens the server to the world and provide access to JupyterLab
-JavaScript objects through the global window variable.
+!! Never use this configuration in production because it
+opens the server to the world and provides access to JupyterLab
+JavaScript objects through the global window variable.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
!! Never use this configuration in production because it
opens the server to the world and provide access to JupyterLab
JavaScript objects through the global window variable.
"""
!! Never use this configuration in production because it
opens the server to the world and provides access to JupyterLab
JavaScript objects through the global window variable.
"""
🤖 Prompt for AI Agents
In ui-tests/jupyter_server_test_config.py around lines 3 to 6, the module
docstring is informal and uses "!!" and inconsistent punctuation; replace it
with a proper triple-quoted docstring sentence starting with a capital letter,
without leading exclamation marks, and ending with a period. Keep the warning
concise (e.g., "Never use this configuration in production because it opens the
server to the world and exposes JupyterLab JavaScript objects via the global
window variable.") and ensure it is the first statement in the file.

from jupyterlab.galata import configure_jupyter_server

configure_jupyter_server(c)

# Uncomment to set server log level to debug level
# c.ServerApp.log_level = "DEBUG"
Comment on lines +7 to +12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

NameError: c is undefined.

The config object c isn’t created before use. This will break jlpm start.

-from jupyterlab.galata import configure_jupyter_server
+from jupyterlab.galata import configure_jupyter_server
+from traitlets.config import get_config
+
+c = get_config()
 
 configure_jupyter_server(c)
 
 # Uncomment to set server log level to debug level
 # c.ServerApp.log_level = "DEBUG"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from jupyterlab.galata import configure_jupyter_server
configure_jupyter_server(c)
# Uncomment to set server log level to debug level
# c.ServerApp.log_level = "DEBUG"
from jupyterlab.galata import configure_jupyter_server
from traitlets.config import get_config
c = get_config()
configure_jupyter_server(c)
# Uncomment to set server log level to debug level
# c.ServerApp.log_level = "DEBUG"
🤖 Prompt for AI Agents
In ui-tests/jupyter_server_test_config.py around lines 7 to 12, the variable `c`
is used before being defined; add the missing import and initialization by
importing get_config from traitlets.config and calling `c = get_config()` before
passing `c` to configure_jupyter_server(c) so the config object exists when
used.

15 changes: 15 additions & 0 deletions ui-tests/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "jupyterlab-deepnote-ui-tests",
"version": "1.0.0",
"description": "JupyterLab jupyterlab-deepnote Integration Tests",
"private": true,
"scripts": {
"start": "jupyter lab --config jupyter_server_test_config.py",
"test": "jlpm playwright test",
"test:update": "jlpm playwright test --update-snapshots"
},
"devDependencies": {
"@jupyterlab/galata": "^5.0.5",
"@playwright/test": "^1.37.0"
}
Comment on lines +6 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Pin test deps to reduce CI drift; add engines and report script.

Caret ranges can cause flaky upgrades. Pin versions and declare runtime expectations.

   "scripts": {
     "start": "jupyter lab --config jupyter_server_test_config.py",
     "test": "jlpm playwright test",
-    "test:update": "jlpm playwright test --update-snapshots"
+    "test:update": "jlpm playwright test --update-snapshots",
+    "report": "jlpm playwright show-report"
   },
+  "engines": {
+    "node": ">=18",
+    "yarn": ">=1.22"
+  },
   "devDependencies": {
-    "@jupyterlab/galata": "^5.0.5",
-    "@playwright/test": "^1.37.0"
+    "@jupyterlab/galata": "5.0.5",
+    "@playwright/test": "1.37.0"
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"scripts": {
"start": "jupyter lab --config jupyter_server_test_config.py",
"test": "jlpm playwright test",
"test:update": "jlpm playwright test --update-snapshots"
},
"devDependencies": {
"@jupyterlab/galata": "^5.0.5",
"@playwright/test": "^1.37.0"
}
"scripts": {
"start": "jupyter lab --config jupyter_server_test_config.py",
"test": "jlpm playwright test",
"test:update": "jlpm playwright test --update-snapshots",
"report": "jlpm playwright show-report"
},
"engines": {
"node": ">=18",
"yarn": ">=1.22"
},
"devDependencies": {
"@jupyterlab/galata": "5.0.5",
"@playwright/test": "1.37.0"
}
🤖 Prompt for AI Agents
In ui-tests/package.json around lines 6 to 14, the devDependencies use caret
ranges and there are no engines or test-report scripts: pin the dependency
versions by removing caret ranges (e.g., set "@jupyterlab/galata": "5.0.5" and
"@playwright/test": "1.37.0"), add an "engines" field declaring supported
runtimes (for example "node": ">=18 <21" and "npm": ">=9" or your project's
supported ranges), and add a script to produce test reports (for example a
"test:report" or "report" script that runs Playwright with --reporter or
generates the HTML report). Ensure the script names and engine ranges match your
CI/runtime policy.

}
14 changes: 14 additions & 0 deletions ui-tests/playwright.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* Configuration for Playwright using default from @jupyterlab/galata
*/
const baseConfig = require('@jupyterlab/galata/lib/playwright-config');

module.exports = {
...baseConfig,
webServer: {
command: 'jlpm start',
url: 'http://localhost:8888/lab',
timeout: 120 * 1000,
reuseExistingServer: !process.env.CI
}
};
23 changes: 23 additions & 0 deletions ui-tests/tests/jupyterlab_deepnote.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { expect, test } from '@jupyterlab/galata';

/**
* Don't load JupyterLab webpage before running the tests.
* This is required to ensure we capture all log messages.
*/
test.use({ autoGoto: false });

test('should emit an activation console message', async ({ page }) => {
const logs: string[] = [];

page.on('console', message => {
logs.push(message.text());
});

await page.goto();

expect(
logs.filter(
s => s === 'JupyterLab extension jupyterlab-deepnote is activated!'
)
).toHaveLength(1);

Check failure on line 22 in ui-tests/tests/jupyterlab_deepnote.spec.ts

View workflow job for this annotation

GitHub Actions / Integration tests

tests/jupyterlab_deepnote.spec.ts:9:5 › should emit an activation console message

1) tests/jupyterlab_deepnote.spec.ts:9:5 › should emit an activation console message ───────────── Error: expect(received).toHaveLength(expected) Expected length: 1 Received length: 0 Received array: [] 20 | s => s === 'JupyterLab extension jupyterlab-deepnote is activated!' 21 | ) > 22 | ).toHaveLength(1); | ^ 23 | }); 24 | at /home/runner/work/jupyterlab-deepnote/jupyterlab-deepnote/ui-tests/tests/jupyterlab_deepnote.spec.ts:22:5
Comment on lines +16 to +22
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Wait for the log to avoid flakiness.

page.goto() can resolve before the activation log is emitted. Poll the in-memory log until it stabilizes.

-  expect(
-    logs.filter(
-      s => s === 'JupyterLab extension jupyterlab-deepnote is activated!'
-    )
-  ).toHaveLength(1);
+  await expect
+    .poll(
+      () =>
+        logs.filter(
+          s => s === 'JupyterLab extension jupyterlab-deepnote is activated!'
+        ).length,
+      { timeout: 10000 }
+    )
+    .toBe(1);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await page.goto();
expect(
logs.filter(
s => s === 'JupyterLab extension jupyterlab-deepnote is activated!'
)
).toHaveLength(1);
await page.goto();
await expect
.poll(
() =>
logs.filter(
s => s === 'JupyterLab extension jupyterlab-deepnote is activated!'
).length,
{ timeout: 10000 }
)
.toBe(1);
🤖 Prompt for AI Agents
In ui-tests/tests/jupyterlab_deepnote.spec.ts around lines 16–22, the test
checks the activation log immediately after page.goto(), which is flaky because
page.goto() can resolve before the activation message is emitted; change the
assertion to poll the in-memory logs until the expected string appears (or until
a timeout) and stabilizes — e.g. use a wait-for helper or page.waitForFunction
that repeatedly evaluates the logs and returns once logs.filter(s => s ===
'JupyterLab extension jupyterlab-deepnote is activated!').length === 1
(optionally ensure no further changes for a short stabilization window) to make
the test deterministic.

});
Empty file added ui-tests/yarn.lock
Empty file.
Loading