Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Galata's setCell helper does not work reliably #15252

Closed
krassowski opened this issue Oct 10, 2023 · 2 comments
Closed

Galata's setCell helper does not work reliably #15252

krassowski opened this issue Oct 10, 2023 · 2 comments

Comments

@krassowski
Copy link
Member

Description

As seen in #15171 galata's setCell helper does not always set cell content correctly. This was also seen in other, older pull requests. A temporary workaround of calling it twice was introduced in fb079b7 which works but is hinting at some problem.

The problematic runs locally exhibited an odd behaviour of switching the code cell type to raw even though this was never requested. It might be that the problem is not in setCell but in setCellType which is the first call of the former:

/**
* Set the input source of a cell
*
* @param cellIndex Cell index
* @param cellType Cell type
* @param source Source
* @returns Action success status
*/
async setCell(
cellIndex: number,
cellType: nbformat.CellType,
source: string
): Promise<boolean> {
if (!(await this.isAnyActive())) {
return false;
}
await this.setCellType(cellIndex, cellType);
if (
!(await this.isCellSelected(cellIndex)) &&
!(await this.selectCells(cellIndex))
) {
return false;
}
await this.enterCellEditingMode(cellIndex);
const keyboard = this.page.keyboard;
await keyboard.press('Control+A');
// give CodeMirror time to style properly
await keyboard.type(source, { delay: cellType === 'code' ? 100 : 0 });
await this.leaveCellEditingMode(cellIndex);
// give CodeMirror time to style properly
if (cellType === 'code') {
await this.page.waitForTimeout(500);
}
return true;
}

Further, from debugging with playwright it seemed that it could have been due to the notebook in panel not being returned at the first attempt leading to incorrect result of cell type null on getCellType check:

async setCellType(
cellIndex: number,
cellType: nbformat.CellType
): Promise<boolean> {
const nbPanel = await this.activity.getPanel();
if (!nbPanel) {
return false;
}
if ((await this.getCellType(cellIndex)) === cellType) {

async getCellType(cellIndex: number): Promise<nbformat.CellType | null> {
const notebook = await this.getNotebookInPanel();
if (!notebook) {
return null;
}

but I am not sure if the runs with instrumentation were representative of what is happening without it.

JupyterLab main head 4.1.0a1

@krassowski krassowski added bug maintenance tag:Testing status:Needs Triage Applied to new issues that need triage and removed status:Needs Triage Applied to new issues that need triage labels Oct 10, 2023
@fcollonval
Copy link
Member

Thanks for digging this one up Mike. I agree on your conclusion that unfortunately instrumentation introduces biases.

@krassowski
Copy link
Member Author

I think this is no longer a problem after recent refactors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants