From 6bfd5a4f17f8901d80108f70af933cbf235a373a Mon Sep 17 00:00:00 2001 From: Nicolas Brichet <32258950+brichet@users.noreply.github.com> Date: Thu, 20 Jun 2024 15:50:59 +0200 Subject: [PATCH] Fix the confusing server errors during UI tests (#1871) * Fix the server errors when renaming directory or removing config file * Clean unexpected error in assignment_list test * Should remove file exists error in create assignment and validate assignments * Try to fix windows test * Remove kernel errors in create assignment ui test, and use galata helpers * Remove kernel errors in validate assignment ui test, and use galata helpers * Fix save notebook when testing notebook application * Set the env variable in package.json instead of task.py, to simplify local tests * Fix notebook tests * Increase the doc test timeout * (1)move config creation in beforeEach, (2)fix closing notebook in jupyterlab tests and (3) remove useless notebook saving * Should remove some more traceback by (1) waiting for a notebook to be ready and (2) killing the kernels on teardown --- .github/workflows/test-docs-python.yml | 2 +- .../tests/ui-tests/assignment_list.spec.ts | 56 ++++--- nbgrader/tests/ui-tests/course_list.spec.ts | 19 ++- ...ment.spec.ts => create_assignment.spec.ts} | 155 +++++++++--------- nbgrader/tests/ui-tests/formgrader.spec.ts | 50 +++--- .../tests/ui-tests/utils/notebook_fixtures.ts | 3 +- ...jupyter_notebook.py => run_jupyter_app.py} | 15 +- .../tests/ui-tests/utils/run_jupyter_lab.py | 27 --- nbgrader/tests/ui-tests/utils/test_utils.ts | 3 +- .../ui-tests/validate_assignment.spec.ts | 73 ++++----- package.json | 5 +- playwright.notebook.config.ts | 5 +- src/index.ts | 6 +- tasks.py | 3 - 14 files changed, 203 insertions(+), 219 deletions(-) rename nbgrader/tests/ui-tests/{create_assignement.spec.ts => create_assignment.spec.ts} (88%) rename nbgrader/tests/ui-tests/utils/{run_jupyter_notebook.py => run_jupyter_app.py} (65%) delete mode 100644 nbgrader/tests/ui-tests/utils/run_jupyter_lab.py diff --git a/.github/workflows/test-docs-python.yml b/.github/workflows/test-docs-python.yml index cf6a7f7dc..53837579c 100644 --- a/.github/workflows/test-docs-python.yml +++ b/.github/workflows/test-docs-python.yml @@ -48,7 +48,7 @@ jobs: run: | if [ "${{ matrix.group }}" == "docs" ]; then echo "GROUP=docs" >> $GITHUB_ENV - echo "TIMEOUT_MINUTES=15" >> $GITHUB_ENV + echo "TIMEOUT_MINUTES=20" >> $GITHUB_ENV fi if [ "${{ matrix.group }}" == "python" ]; then echo "GROUP=python" >> $GITHUB_ENV diff --git a/nbgrader/tests/ui-tests/assignment_list.spec.ts b/nbgrader/tests/ui-tests/assignment_list.spec.ts index ed9f5ddf0..42ad38037 100644 --- a/nbgrader/tests/ui-tests/assignment_list.spec.ts +++ b/nbgrader/tests/ui-tests/assignment_list.spec.ts @@ -66,6 +66,14 @@ test.beforeEach(async ({ request, tmpPath }) => { await contents.createDirectory(tmpPath); + if (await contents.fileExists("nbgrader_config.py")){ + await contents.deleteFile("nbgrader_config.py"); + } + await contents.uploadFile( + path.resolve(__dirname, "./files/nbgrader_config.py"), + "nbgrader_config.py" + ); + if (!isWindows) { exchange_dir = fs.mkdtempSync( path.join(os.tmpdir(), "nbgrader_exchange_test_") @@ -77,29 +85,26 @@ test.beforeEach(async ({ request, tmpPath }) => { /* * delete temp directories at the end of test */ -test.afterEach(async ({ request, tmpPath }) => { +test.afterEach(async ({ request, page, tmpPath }) => { if (!isWindows) { fs.rmSync(exchange_dir, { recursive: true, force: true }); fs.rmSync(cache_dir, { recursive: true, force: true }); } if (request === undefined) throw new Error("Request is undefined."); - const contents = galata.newContentsHelper(request); + const contents = galata.newContentsHelper(request, page); await contents.deleteDirectory(tmpPath); - - if (await contents.fileExists("nbgrader_config.py")) - contents.deleteFile("nbgrader_config.py"); - contents.uploadFile( - path.resolve(__dirname, "./files/nbgrader_config.py"), - "nbgrader_config.py" - ); }); /* * Create a nbgrader file system and modify config */ -const addCourses = async (request: APIRequestContext, tmpPath: string) => { - const contents = galata.newContentsHelper(request); +const addCourses = async ( + request: APIRequestContext, + page: IJupyterLabPageFixture, + tmpPath: string +) => { + const contents = galata.newContentsHelper(request, page); // copy files from the user guide const source = path.resolve( @@ -235,6 +240,15 @@ test("Open assignment list tab from menu", async ({ page, tmpPath }) => { if (isNotebook) await page.goto(`tree/${tmpPath}`); + // Creating the config file is useful to avoid server error about the exchange + // directory not writable. + // This is not required for the test, only to have cleaner logs. + await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); + fs.copyFileSync( + path.resolve(testDir, "nbgrader_config.py"), + path.resolve(testDir, tmpPath, "nbgrader_config.py") + ); + const nbgrader_menu = page.locator(`${menuPanelId} div.lm-MenuBar-itemLabel:text("Nbgrader")`); const assignmentList_menu = page.locator( '#jp-mainmenu-nbgrader li[data-command="nbgrader:open-assignment-list"]' @@ -277,7 +291,7 @@ test("Show assignment list", async ({ page, request, tmpPath }) => { if (isNotebook) await page.goto(`tree/${tmpPath}`); await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openAssignmentList(page); // Wait for DOM of each status @@ -311,7 +325,7 @@ test("Multiple released assignments", async ({ page, request, tmpPath }) => { if (isNotebook) await page.goto(`tree/${tmpPath}`); await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openAssignmentList(page); // release two assignments @@ -347,7 +361,7 @@ test("Fetch assignments", async ({ page, request, tmpPath }) => { if (isNotebook) await page.goto(`tree/${tmpPath}`); await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openAssignmentList(page); // release some assignments @@ -400,7 +414,7 @@ test("Submit assignments", async ({ page, request, tmpPath }) => { // create directories and config files, and open assignment_list tab await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openAssignmentList(page); // release some assignments @@ -468,7 +482,7 @@ test("submit assignment missing notebook", async ({ // create directories and config files, and open assignment_list tab await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openAssignmentList(page); // release some assignments @@ -556,7 +570,7 @@ test("Fetch a second assignment", async ({ page, request, tmpPath }) => { if (isNotebook) await page.goto(`tree/${tmpPath}`); await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openAssignmentList(page); // release some assignments @@ -627,7 +641,7 @@ test("Submit another assignments", async ({ page, request, tmpPath }) => { // create directories and config files, and open assignment_list tab await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openAssignmentList(page); // release some assignments @@ -681,7 +695,7 @@ test("Validate OK", async ({ page, request, tmpPath }) => { // create directories and config files, and open assignment_list tab await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openAssignmentList(page); // release some assignments @@ -731,7 +745,7 @@ test("Validate failure", async ({ page, request, tmpPath }) => { // create directories and config files, and open assignment_list tab await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openAssignmentList(page); // release some assignments @@ -785,7 +799,7 @@ test("Missing exchange directory", async ({ page, request, tmpPath }) => { // create directories and config files await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); // delete exchange directory fs.rmSync(exchange_dir, { recursive: true, force: true }); diff --git a/nbgrader/tests/ui-tests/course_list.spec.ts b/nbgrader/tests/ui-tests/course_list.spec.ts index 8a3588d9c..31b18a942 100644 --- a/nbgrader/tests/ui-tests/course_list.spec.ts +++ b/nbgrader/tests/ui-tests/course_list.spec.ts @@ -60,6 +60,14 @@ test.beforeEach(async ({ request, tmpPath }) => { await contents.createDirectory(tmpPath); + if (await contents.fileExists("nbgrader_config.py")){ + await contents.deleteFile("nbgrader_config.py"); + } + await contents.uploadFile( + path.resolve(__dirname, "./files/nbgrader_config.py"), + "nbgrader_config.py" + ); + if (!isWindows) { exchange_dir = fs.mkdtempSync( path.join(os.tmpdir(), "nbgrader_exchange_test_") @@ -71,22 +79,15 @@ test.beforeEach(async ({ request, tmpPath }) => { /* * delete temp directories at the end of test */ -test.afterEach(async ({ request, tmpPath }) => { +test.afterEach(async ({ request, page, tmpPath }) => { if (request === undefined) throw new Error("Request is undefined."); - const contents = galata.newContentsHelper(request); + const contents = galata.newContentsHelper(request, page); await contents.deleteDirectory(tmpPath); if (!isWindows) { fs.rmSync(exchange_dir, { recursive: true, force: true }); fs.rmSync(cache_dir, { recursive: true, force: true }); } - - if (await contents.fileExists("nbgrader_config.py")) - contents.deleteFile("nbgrader_config.py"); - contents.uploadFile( - path.resolve(__dirname, "./files/nbgrader_config.py"), - "nbgrader_config.py" - ); }); /* diff --git a/nbgrader/tests/ui-tests/create_assignement.spec.ts b/nbgrader/tests/ui-tests/create_assignment.spec.ts similarity index 88% rename from nbgrader/tests/ui-tests/create_assignement.spec.ts rename to nbgrader/tests/ui-tests/create_assignment.spec.ts index 0bb8b4bc7..dc32183ba 100644 --- a/nbgrader/tests/ui-tests/create_assignement.spec.ts +++ b/nbgrader/tests/ui-tests/create_assignment.spec.ts @@ -48,6 +48,17 @@ test.beforeEach(async ({ request, tmpPath }) => { if (request === undefined) throw new Error("Request is undefined."); const contents = galata.newContentsHelper(request); + + if (await contents.fileExists("nbgrader_config.py")) { + await contents.deleteFile("nbgrader_config.py"); + } + await contents.uploadFile( + path.resolve(__dirname, "./files/nbgrader_config.py"), + "nbgrader_config.py" + ); + + await contents.createDirectory(tmpPath); + nbFiles.forEach((elem) => { contents.uploadFile( path.resolve(__dirname, `./files/${elem}`), @@ -59,55 +70,34 @@ test.beforeEach(async ({ request, tmpPath }) => { /* * Delete temp directory at the end of test. */ -test.afterAll(async ({ request, tmpPath }) => { +test.afterEach(async ({ request, page, tmpPath }) => { if (request === undefined) throw new Error("Request is undefined."); - const contents = galata.newContentsHelper(request); - await contents.deleteDirectory(tmpPath); + if (!isNotebook) { + // Close opened notebook. + while (await page.notebook.isAnyActive()) { + await page.notebook.close(); + } + } + await page.kernel.shutdownAll(); - if (await contents.fileExists("nbgrader_config.py")) - contents.deleteFile("nbgrader_config.py"); - contents.uploadFile( - path.resolve(__dirname, "./files/nbgrader_config.py"), - "nbgrader_config.py" - ); + const contents = galata.newContentsHelper(request, page); + await contents.deleteDirectory(tmpPath); }); -/* - * Open a notebook file. - * NOTES: - * This function is only useful if testing extension in JupyterLab. - * In Notebook tests we open a new browser tab instead. +/** + * Save the current active notebook. + * TODO: use only the page.notebook helper when it work with notebook. */ -const openNotebook = async (page: IJupyterLabPageFixture, notebook: string) => { - var filename = notebook + ".ipynb"; - var tab_count = await page - .locator("#jp-main-dock-panel .lm-TabBar-tab") - .count(); - await page - .locator( - `#filebrowser .jp-DirListing-content .jp-DirListing-itemText span:text-is('${filename}')` - ) - .dblclick(); - await expect(page.locator("#jp-main-dock-panel .lm-TabBar-tab")).toHaveCount( - tab_count + 1 - ); - await page.locator(".jp-Notebook-cell").waitFor(); -}; - -/* - * Save the current notebook file. - */ -const saveCurrentNotebook = async (page: IJupyterLabPageFixture) => { - return await page.evaluate(async () => { - var nb = window.jupyterapp.shell.currentWidget as NotebookPanel; - await nb.context.save(); - }); - - // TODO : ensure metadata has been saved - // Read local file ? -}; - +const saveNotebook = async (page: IJupyterLabPageFixture): Promise => { + if (isNotebook) { + await page.evaluate(async () => { + await window.galata.saveActiveNotebook(); + }); + } else { + await page.notebook.save(); + } +} /* * Activate assignment toolbar. */ @@ -232,7 +222,8 @@ test("manual cell", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/blank.ipynb`); } else { - await openNotebook(page, "blank"); + await page.notebook.open("blank.ipynb"); + await page.notebook.activate("blank.ipynb"); } await activateToolbar(page); @@ -259,12 +250,10 @@ test("manual cell", async ({ page, tmpPath }) => { await setId(page); expect(await getCellMetadata(page)).toHaveProperty("grade_id", "foo"); - await saveCurrentNotebook(page); + await saveNotebook(page); await selectInToolbar(page, ""); expect(await getCellMetadata(page)).toBeUndefined(); - - await saveCurrentNotebook(page); }); /* @@ -275,7 +264,8 @@ test("task cell", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/task.ipynb`); } else { - await openNotebook(page, "task"); + await page.notebook.open("task.ipynb"); + await page.notebook.activate("task.ipynb"); } await activateToolbar(page); @@ -301,12 +291,10 @@ test("task cell", async ({ page, tmpPath }) => { await setId(page); expect(await getCellMetadata(page)).toHaveProperty("grade_id", "foo"); - await saveCurrentNotebook(page); + await saveNotebook(page); await selectInToolbar(page, ""); expect(await getCellMetadata(page)).toBeUndefined(); - - await saveCurrentNotebook(page); }); /* @@ -317,7 +305,8 @@ test("solution cell", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/blank.ipynb`); } else { - await openNotebook(page, "blank"); + await page.notebook.open("blank.ipynb"); + await page.notebook.activate("blank.ipynb"); } await activateToolbar(page); @@ -339,12 +328,10 @@ test("solution cell", async ({ page, tmpPath }) => { await setId(page); expect(await getCellMetadata(page)).toHaveProperty("grade_id", "foo"); - await saveCurrentNotebook(page); + await saveNotebook(page); await selectInToolbar(page, ""); expect(await getCellMetadata(page)).toBeUndefined(); - - await saveCurrentNotebook(page); }); /* @@ -355,7 +342,8 @@ test("tests cell", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/blank.ipynb`); } else { - await openNotebook(page, "blank"); + await page.notebook.open("blank.ipynb"); + await page.notebook.activate("blank.ipynb"); } await activateToolbar(page); @@ -382,12 +370,10 @@ test("tests cell", async ({ page, tmpPath }) => { await setId(page); expect(await getCellMetadata(page)).toHaveProperty("grade_id", "foo"); - await saveCurrentNotebook(page); + await saveNotebook(page); await selectInToolbar(page, ""); expect(await getCellMetadata(page)).toBeUndefined(); - - await saveCurrentNotebook(page); }); /* @@ -398,7 +384,8 @@ test("tests to solution cell", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/blank.ipynb`); } else { - await openNotebook(page, "blank"); + await page.notebook.open("blank.ipynb"); + await page.notebook.activate("blank.ipynb"); } await activateToolbar(page); @@ -425,7 +412,7 @@ test("tests to solution cell", async ({ page, tmpPath }) => { await setId(page); expect(await getCellMetadata(page)).toHaveProperty("grade_id", "foo"); - await saveCurrentNotebook(page); + await saveNotebook(page); await selectInToolbar(page, "solution"); var metadata = await getCellMetadata(page); @@ -433,11 +420,10 @@ test("tests to solution cell", async ({ page, tmpPath }) => { expect(metadata).toHaveProperty("grade", false); expect(metadata).toHaveProperty("locked", false); expect(metadata["points"]).toBeUndefined(); - await saveCurrentNotebook(page); + await saveNotebook(page); await selectInToolbar(page, ""); expect(await getCellMetadata(page)).toBeUndefined(); - await saveCurrentNotebook(page); }); /* @@ -448,7 +434,8 @@ test("locked cell", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/blank.ipynb`); } else { - await openNotebook(page, "blank"); + await page.notebook.open("blank.ipynb"); + await page.notebook.activate("blank.ipynb"); } await activateToolbar(page); @@ -471,11 +458,10 @@ test("locked cell", async ({ page, tmpPath }) => { await setId(page); expect(await getCellMetadata(page)).toHaveProperty("grade_id", "foo"); - await saveCurrentNotebook(page); + await saveNotebook(page); await selectInToolbar(page, ""); expect(await getCellMetadata(page)).toBeUndefined(); - await saveCurrentNotebook(page); }); /* @@ -486,7 +472,8 @@ test("tab key", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/blank.ipynb`); } else { - await openNotebook(page, "blank"); + await page.notebook.open("blank.ipynb"); + await page.notebook.activate("blank.ipynb"); } await activateToolbar(page); @@ -515,7 +502,8 @@ test("total points", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/blank.ipynb`); } else { - await openNotebook(page, "blank"); + await page.notebook.open("blank.ipynb"); + await page.notebook.activate("blank.ipynb"); } await activateToolbar(page); @@ -569,7 +557,8 @@ test("task total points", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/task.ipynb`); } else { - await openNotebook(page, "task"); + await page.notebook.open("task.ipynb"); + await page.notebook.activate("task.ipynb"); } await activateToolbar(page); @@ -623,7 +612,8 @@ test("cell ids", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/blank.ipynb`); } else { - await openNotebook(page, "blank"); + await page.notebook.open("blank.ipynb"); + await page.notebook.activate("blank.ipynb"); } await activateToolbar(page); @@ -633,7 +623,7 @@ test("cell ids", async ({ page, tmpPath }) => { await setId(page, ""); // wait for error on saving with empty id - await saveCurrentNotebook(page); + await saveNotebook(page); await waitForErrorModal(page); await closeErrorModal(page); @@ -648,7 +638,7 @@ test("cell ids", async ({ page, tmpPath }) => { await setId(page, "foo", 1); // wait for error on saving with empty id - await saveCurrentNotebook(page); + await saveNotebook(page); await waitForErrorModal(page); await closeErrorModal(page); }); @@ -661,7 +651,8 @@ test("task cell ids", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/task.ipynb`); } else { - await openNotebook(page, "task"); + await page.notebook.open("task.ipynb"); + await page.notebook.activate("task.ipynb"); } await activateToolbar(page); @@ -671,7 +662,7 @@ test("task cell ids", async ({ page, tmpPath }) => { await setId(page, ""); // wait for error on saving with empty id - await saveCurrentNotebook(page); + await saveNotebook(page); await waitForErrorModal(page); await closeErrorModal(page); @@ -686,7 +677,7 @@ test("task cell ids", async ({ page, tmpPath }) => { await setId(page, "foo", 1); // wait for error on saving with empty id - await saveCurrentNotebook(page); + await saveNotebook(page); await waitForErrorModal(page); await closeErrorModal(page); }); @@ -699,7 +690,8 @@ test("negative points", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/blank.ipynb`); } else { - await openNotebook(page, "blank"); + await page.notebook.open("blank.ipynb"); + await page.notebook.activate("blank.ipynb"); } await activateToolbar(page); @@ -728,7 +720,8 @@ test("task negative points", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/task.ipynb`); } else { - await openNotebook(page, "task"); + await page.notebook.open("task.ipynb"); + await page.notebook.activate("task.ipynb"); } await activateToolbar(page); @@ -756,7 +749,8 @@ test("schema version", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/old-schema.ipynb`); } else { - await openNotebook(page, "old-schema"); + await page.notebook.open("old-schema.ipynb"); + await page.notebook.activate("old-schema.ipynb"); } // activate toolbar should show an error modal @@ -773,7 +767,8 @@ test("invalid nbgrader cell type", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/blank.ipynb`); } else { - await openNotebook(page, "blank"); + await page.notebook.open("blank.ipynb"); + await page.notebook.activate("blank.ipynb"); } await activateToolbar(page); @@ -794,7 +789,7 @@ test("invalid nbgrader cell type", async ({ page, tmpPath }) => { await setId(page); expect(await getCellMetadata(page)).toHaveProperty("grade_id", "foo"); - await saveCurrentNotebook(page); + await saveNotebook(page); // change the cell to markdown await page.locator(".jp-Cell .jp-InputArea-prompt").first().click(); diff --git a/nbgrader/tests/ui-tests/formgrader.spec.ts b/nbgrader/tests/ui-tests/formgrader.spec.ts index 4f74d9900..a16037962 100644 --- a/nbgrader/tests/ui-tests/formgrader.spec.ts +++ b/nbgrader/tests/ui-tests/formgrader.spec.ts @@ -21,7 +21,7 @@ const tempPath = 'nbgrader-formgrader-test'; let test = jupyterLabTest; let mainPanelId = '#jp-main-dock-panel'; -let menuPanelId = '#jp-menu-panel'; +let menuPanelId = '#jp-MainMenu'; let mainPanelTabCount = 1; const baseTestUse = { @@ -41,7 +41,6 @@ if (isNotebook) { autoGoto: false }); mainPanelId = '#main-panel'; - menuPanelId = '#menu-panel'; mainPanelTabCount = 2; } else { @@ -61,6 +60,14 @@ test.beforeEach(async ({ request, tmpPath }) => { await contents.createDirectory(tmpPath); + if (await contents.fileExists("nbgrader_config.py")){ + await contents.deleteFile("nbgrader_config.py"); + } + await contents.uploadFile( + path.resolve(__dirname, "./files/nbgrader_config.py"), + "nbgrader_config.py" + ); + if (!isWindows) { exchange_dir = fs.mkdtempSync( path.join(os.tmpdir(), "nbgrader_exchange_test_") @@ -72,7 +79,7 @@ test.beforeEach(async ({ request, tmpPath }) => { /* * delete temp directories at the end of test */ -test.afterEach(async ({ request, tmpPath }) => { +test.afterEach(async ({ request, page, tmpPath }) => { if (!isWindows) { fs.rmSync(exchange_dir, { recursive: true, force: true }); fs.rmSync(cache_dir, { recursive: true, force: true }); @@ -80,22 +87,19 @@ test.afterEach(async ({ request, tmpPath }) => { if (request === undefined) throw new Error("Request is undefined."); - const contents = galata.newContentsHelper(request); + const contents = galata.newContentsHelper(request, page); await contents.deleteDirectory(tmpPath); - - if (await contents.fileExists("nbgrader_config.py")) - contents.deleteFile("nbgrader_config.py"); - contents.uploadFile( - path.resolve(__dirname, "./files/nbgrader_config.py"), - "nbgrader_config.py" - ); }); /* * Create a nbgrader file system */ -const addCourses = async (request: APIRequestContext, tmpPath: string) => { - const contents = galata.newContentsHelper(request); +const addCourses = async ( + request: APIRequestContext, + page: IJupyterLabPageFixture, + tmpPath: string +) => { + const contents = galata.newContentsHelper(request, page); // copy files from the user guide const source_path = path.resolve( @@ -342,7 +346,7 @@ test("Load manage assignments", async ({ page, baseURL, request, tmpPath }) => { // create environment await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openFormgrader(page); // get formgrader iframe and check for breadcrumbs @@ -394,7 +398,7 @@ test("Load manage submissions", async ({ page, baseURL, request, tmpPath }) => { // create environment await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openFormgrader(page); // get formgrader iframe @@ -455,7 +459,7 @@ test("Load gradebook1", async ({ page, baseURL, request, tmpPath }) => { // create environment await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openFormgrader(page); // get formgrader iframe @@ -490,7 +494,7 @@ test("Load gradebook2", async ({ page, baseURL, request, tmpPath }) => { // create environment await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openFormgrader(page); // get formgrader iframe @@ -541,7 +545,7 @@ test("Load gradebook3", async ({ page, baseURL, request, tmpPath }) => { // create environment await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openFormgrader(page); // get formgrader iframe @@ -632,7 +636,7 @@ test("Gradebook3 show hide names", async ({ // create environment await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openFormgrader(page); // get formgrader iframe @@ -686,7 +690,7 @@ test('Gradebook toggle names button', async ({ // create environment await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openFormgrader(page); // get formgrader iframe @@ -736,7 +740,7 @@ test("Load students", async ({ page, baseURL, request, tmpPath }) => { // create environment await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openFormgrader(page); // get formgrader iframe @@ -784,7 +788,7 @@ test("Load students submissions", async ({ // create environment await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openFormgrader(page); // get formgrader iframe @@ -829,7 +833,7 @@ test("Switch views", async ({ page, baseURL, request, tmpPath }) => { // create environment await createEnv(testDir, tmpPath, exchange_dir, cache_dir, isWindows); - await addCourses(request, tmpPath); + await addCourses(request, page, tmpPath); await openFormgrader(page); // get formgrader iframe diff --git a/nbgrader/tests/ui-tests/utils/notebook_fixtures.ts b/nbgrader/tests/ui-tests/utils/notebook_fixtures.ts index 0a4ccf37f..5ca2a335a 100644 --- a/nbgrader/tests/ui-tests/utils/notebook_fixtures.ts +++ b/nbgrader/tests/ui-tests/utils/notebook_fixtures.ts @@ -1,8 +1,9 @@ import { test as base } from '@jupyterlab/galata'; +import { Page } from '@playwright/test'; export const test = base.extend({ waitForApplication: async ({ baseURL }, use, testInfo) => { - const waitIsReady = async (page): Promise => { + const waitIsReady = async (page: Page): Promise => { await page.locator('#main-panel').waitFor(); }; await use(waitIsReady); diff --git a/nbgrader/tests/ui-tests/utils/run_jupyter_notebook.py b/nbgrader/tests/ui-tests/utils/run_jupyter_app.py similarity index 65% rename from nbgrader/tests/ui-tests/utils/run_jupyter_notebook.py rename to nbgrader/tests/ui-tests/utils/run_jupyter_app.py index 550d2d1c7..71629f5ef 100644 --- a/nbgrader/tests/ui-tests/utils/run_jupyter_notebook.py +++ b/nbgrader/tests/ui-tests/utils/run_jupyter_app.py @@ -12,16 +12,21 @@ if not os.path.isdir(env["NBGRADER_TEST_DIR"]): raise Exception(f"Test directory {env['NBGRADER_TEST_DIR']} doesn't exists") -shutil.copyfile("./nbgrader/tests/ui-tests/utils/jupyter_server_config_notebook.py", - join(root_dir, "jupyter_server_config_notebook.py")) +subprocess.check_call([sys.executable, "-m", "jupyter", "server", "extension", "enable", "--user", "--py", "nbgrader"], env=env) +subprocess.check_call([sys.executable, "-m", "jupyter", "labextension", "develop", "--overwrite", "."]) shutil.copyfile("./nbgrader/tests/ui-tests/files/nbgrader_config.py", join(root_dir, "nbgrader_config.py")) -subprocess.check_call([sys.executable, "-m", "jupyter", "server", "extension", "enable", "--user", "--py", "nbgrader"], env=env) -subprocess.check_call([sys.executable, "-m", "jupyter", "labextension", "develop", "--overwrite", "."]) +if "NBGRADER_TEST_IS_NOTEBOOK" in env.keys(): + shutil.copyfile("./nbgrader/tests/ui-tests/utils/jupyter_server_config_notebook.py", + join(root_dir, "jupyter_server_config_notebook.py")) + command = ["jupyter", "notebook", "--config", "./jupyter_server_config_notebook.py"] +else: + shutil.copyfile("./nbgrader/tests/ui-tests/utils/jupyter_server_config.py", + join(root_dir, "jupyter_server_config.py")) + command = ["jupyter", "lab", "--config", "./jupyter_server_config.py"] os.chdir(root_dir) -command = ["jupyter", "notebook", "--config", "./jupyter_server_config_notebook.py"] subprocess.run(command) diff --git a/nbgrader/tests/ui-tests/utils/run_jupyter_lab.py b/nbgrader/tests/ui-tests/utils/run_jupyter_lab.py deleted file mode 100644 index dc86b44df..000000000 --- a/nbgrader/tests/ui-tests/utils/run_jupyter_lab.py +++ /dev/null @@ -1,27 +0,0 @@ -import sys -import os -from os.path import join -import shutil -import subprocess - -env = os.environ.copy() -root_dir = env.get("NBGRADER_TEST_DIR", '') - -if not env["NBGRADER_TEST_DIR"]: - raise Exception("Test directory not provided") -if not os.path.isdir(env["NBGRADER_TEST_DIR"]): - raise Exception(f"Test directory {env['NBGRADER_TEST_DIR']} doesn't exists") - -shutil.copyfile("./nbgrader/tests/ui-tests/utils/jupyter_server_config.py", - join(root_dir, "jupyter_server_config.py")) - -shutil.copyfile("./nbgrader/tests/ui-tests/files/nbgrader_config.py", - join(root_dir, "nbgrader_config.py")) - -subprocess.check_call([sys.executable, "-m", "jupyter", "server", "extension", "enable", "--user", "--py", "nbgrader"], env=env) -subprocess.check_call([sys.executable, "-m", "jupyter", "labextension", "develop", "--overwrite", "."]) - -os.chdir(root_dir) - -command = ["jupyter", "lab", "--config", "./jupyter_server_config.py"] -subprocess.run(command) diff --git a/nbgrader/tests/ui-tests/utils/test_utils.ts b/nbgrader/tests/ui-tests/utils/test_utils.ts index 9b45b6618..2300282f3 100644 --- a/nbgrader/tests/ui-tests/utils/test_utils.ts +++ b/nbgrader/tests/ui-tests/utils/test_utils.ts @@ -59,12 +59,11 @@ c.Exchange.assignment_dir = r"${path.resolve(rootDir, tmpPath)}" } /* Fill database */ - await executeCommand("nbgrader db assignment add 'Problem Set 1'"); + await executeCommand("nbgrader db assignment add \"Problem Set 1\""); await executeCommand("nbgrader db assignment add ps.01"); await executeCommand("nbgrader db student add Bitdiddle --first-name Ben --last-name B"); await executeCommand("nbgrader db student add Hacker --first-name Alyssa --last-name H"); await executeCommand("nbgrader db student add Reasoner --first-name Louis --last-name R"); - } /* diff --git a/nbgrader/tests/ui-tests/validate_assignment.spec.ts b/nbgrader/tests/ui-tests/validate_assignment.spec.ts index e3ec302e2..c84ea8e90 100644 --- a/nbgrader/tests/ui-tests/validate_assignment.spec.ts +++ b/nbgrader/tests/ui-tests/validate_assignment.spec.ts @@ -63,6 +63,17 @@ test.beforeEach(async ({ request, tmpPath }) => { if (request === undefined) throw new Error("Request is undefined."); const contents = galata.newContentsHelper(request); + + if (await contents.fileExists("nbgrader_config.py")) { + await contents.deleteFile("nbgrader_config.py"); + } + await contents.uploadFile( + path.resolve(__dirname, "./files/nbgrader_config.py"), + "nbgrader_config.py" + ); + + await contents.createDirectory(tmpPath); + nbFiles.forEach((elem) => { contents.uploadFile( path.resolve(__dirname, `./files/${elem}`), @@ -74,42 +85,21 @@ test.beforeEach(async ({ request, tmpPath }) => { /* * Delete temp directory at the end of test */ -test.afterAll(async ({ request, tmpPath }) => { +test.afterEach(async ({ request, page, tmpPath }) => { if (request === undefined) throw new Error("Request is undefined."); - const contents = galata.newContentsHelper(request); - await contents.deleteDirectory(tmpPath); + if (!isNotebook) { + // Close opened notebook. + while (await page.notebook.isAnyActive()) { + await page.notebook.close(); + } + } + await page.kernel.shutdownAll(); - if (await contents.fileExists("nbgrader_config.py")) - contents.deleteFile("nbgrader_config.py"); - contents.uploadFile( - path.resolve(__dirname, "./files/nbgrader_config.py"), - "nbgrader_config.py" - ); + const contents = galata.newContentsHelper(request, page); + await contents.deleteDirectory(tmpPath); }); -/* - * Open a notebook file. - * NOTES: - * This function is only useful if testing extension in JupyterLab. - * In Notebook tests we open a new browser tab instead. - */ -const openNotebook = async (page: IJupyterLabPageFixture, notebook: string) => { - var filename = notebook + ".ipynb"; - var tab_count = await page - .locator("#jp-main-dock-panel .lm-TabBar-tab") - .count(); - await page - .locator( - `#filebrowser .jp-DirListing-content .jp-DirListing-itemText span:text-is('${filename}')` - ) - .dblclick(); - await expect(page.locator("#jp-main-dock-panel .lm-TabBar-tab")).toHaveCount( - tab_count + 1 - ); - await page.locator(".jp-Notebook-cell").first().waitFor(); -}; - /* * Test validation success */ @@ -119,7 +109,8 @@ test("Validation success", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/submitted-changed.ipynb`); } else { - await openNotebook(page, "submitted-changed"); + await page.notebook.open("submitted-changed.ipynb"); + await page.notebook.activate("submitted-changed.ipynb"); } // click on validate, and expect a success modal @@ -139,7 +130,8 @@ test("Validation failure", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/submitted-unchanged.ipynb`); } else { - await openNotebook(page, "submitted-unchanged"); + await page.notebook.open("submitted-unchanged.ipynb"); + await page.notebook.activate("submitted-unchanged.ipynb"); } // click on validate, and expect an error modal @@ -161,7 +153,8 @@ test("Validation grade cell changed", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/submitted-grade-cell-changed.ipynb`); } else { - await openNotebook(page, "submitted-grade-cell-changed"); + await page.notebook.open("submitted-grade-cell-changed.ipynb"); + await page.notebook.activate("submitted-grade-cell-changed.ipynb"); } // click on validate, and expect an error modal @@ -183,7 +176,8 @@ test("Validation locked cell changed", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/submitted-locked-cell-changed.ipynb`); } else { - await openNotebook(page, "submitted-locked-cell-changed"); + await page.notebook.open("submitted-locked-cell-changed.ipynb"); + await page.notebook.activate("submitted-locked-cell-changed.ipynb"); } // click on validate, and expect an error modal @@ -205,7 +199,8 @@ test("Validation open relative file", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/open_relative_file.ipynb`); } else { - await openNotebook(page, "open_relative_file"); + await page.notebook.open("open_relative_file.ipynb"); + await page.notebook.activate("open_relative_file.ipynb"); } // click on validate, and expect a success modal @@ -227,7 +222,8 @@ test("Validation grade cell type changed", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/submitted-grade-cell-type-changed.ipynb`); } else { - await openNotebook(page, "submitted-grade-cell-type-changed"); + await page.notebook.open("submitted-grade-cell-type-changed.ipynb"); + await page.notebook.activate("submitted-grade-cell-type-changed.ipynb"); } // click on validate, and expect an error modal @@ -249,7 +245,8 @@ test("Validation answer cell type changed", async ({ page, tmpPath }) => { if (isNotebook) { await page.goto(`notebooks/${tmpPath}/submitted-answer-cell-type-changed.ipynb`); } else { - await openNotebook(page, "submitted-answer-cell-type-changed"); + await page.notebook.open("submitted-answer-cell-type-changed.ipynb"); + await page.notebook.activate("submitted-answer-cell-type-changed.ipynb"); } // click on validate, and expect an error modal diff --git a/package.json b/package.json index 8a4aac57c..31f45217c 100644 --- a/package.json +++ b/package.json @@ -35,10 +35,9 @@ "clean:labextension": "rimraf nbgrader/labextension", "clean:all": "jlpm clean:lib && jlpm clean:labextension", "install:labextension": "jlpm build", - "start:test": "python ./nbgrader/tests/ui-tests/utils/run_jupyter_lab.py", - "start:test:notebook": "python ./nbgrader/tests/ui-tests/utils/run_jupyter_notebook.py", + "start:test": "python ./nbgrader/tests/ui-tests/utils/run_jupyter_app.py", "test": "jlpm playwright test", - "test:notebook": "jlpm playwright test --config=playwright.notebook.config.ts", + "test:notebook": "NBGRADER_TEST_IS_NOTEBOOK=1 jlpm playwright test --config=playwright.notebook.config.ts", "watch": "run-p watch:src watch:labextension", "watch:src": "tsc -w", "watch:labextension": "jupyter labextension watch ." diff --git a/playwright.notebook.config.ts b/playwright.notebook.config.ts index 15e8d8a85..ba9e2d57f 100644 --- a/playwright.notebook.config.ts +++ b/playwright.notebook.config.ts @@ -13,10 +13,9 @@ module.exports = { appPath: '', }, webServer: { - command: 'jlpm start:test:notebook', + command: 'jlpm start:test', url: 'http://localhost:8888/tree', timeout: 120 * 1000, reuseExistingServer: !process.env.CI, - }, - + } }; diff --git a/src/index.ts b/src/index.ts index 5b8d76785..3f500a9cb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -47,9 +47,9 @@ const availableExtensionsManager: JupyterFrontEndPlugin = { activate: ( app: JupyterFrontEnd, mainMenu: IMainMenu, - palette: ICommandPalette, - labShell: ILabShell, - notebookShell: INotebookShell + palette: ICommandPalette | null, + labShell: ILabShell | null, + notebookShell: INotebookShell | null ) => { let mainExtensions = false; diff --git a/tasks.py b/tasks.py index b617b29a2..46906a5d8 100755 --- a/tasks.py +++ b/tasks.py @@ -82,9 +82,6 @@ def _run_ts_test(args, notebook=False): root_dir = mkdtemp(prefix="nbgrader-galata-") os.environ["NBGRADER_TEST_DIR"] = root_dir - if notebook: - os.environ["NBGRADER_TEST_IS_NOTEBOOK"] = "1" - cmd = ['jlpm', f'test{":notebook" if notebook else ""}', '--retries=2'] + args run(" ".join(cmd))