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.

Sign up for GitHub

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

ci: skip pr-comment workflow on closed PRs #5979

Closed
wants to merge 4 commits into from
Closed

ci: skip pr-comment workflow on closed PRs #5979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
90e7c84
flake fixes
midleman Jan 13, 2025
af60103
Merge branch 'main' into mi/fix-notebook-create-flakes
midleman Jan 13, 2025
8e3295f
stepWrapper
midleman Jan 13, 2025
216a220
skip job on closed
midleman Jan 13, 2025
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
1 change: 1 addition & 0 deletions .github/workflows/pr-e2e-comment.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ on:
jobs:
e2e-tags:
runs-on: ubuntu-latest
if: ${{ github.event.pull_request.state != 'closed' }} # Skip job if PR is closed
steps:
- uses: actions/checkout@v4

Expand Down
97 changes: 58 additions & 39 deletions test/e2e/pages/notebooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Code } from '../infra/code';
import { QuickInput } from './quickInput';
import { QuickAccess } from './quickaccess';
import { basename } from 'path';
import { expect } from '@playwright/test';
import test, { expect } from '@playwright/test';


const KERNEL_LABEL = '.kernel-label';
Expand Down Expand Up @@ -37,24 +37,33 @@ export class Notebooks {
constructor(private code: Code, private quickinput: QuickInput, private quickaccess: QuickAccess) { }

async selectInterpreter(kernelGroup: string, desiredKernel: string) {
await expect(this.notebookProgressBar).not.toBeVisible({ timeout: 30000 });
await expect(this.code.driver.page.locator(DETECTING_KERNELS_TEXT)).not.toBeVisible({ timeout: 30000 });

// Wait for either "select kernel" or "the desired kernel" to appear in KERNEL_LABEL
const kernelRegex = new RegExp(`${SELECT_KERNEL_TEXT}|${desiredKernel}`);
const kernelLabelLocator = this.code.driver.page.locator(KERNEL_LABEL);
await expect(kernelLabelLocator).toHaveText(kernelRegex, { timeout: 10000 });

// Retrieve the matched text for conditional logic
const matchedText = await kernelLabelLocator.textContent() || '';

if (!new RegExp(desiredKernel).test(matchedText)) {
await this.code.driver.page.locator(KERNEL_ACTION).click();
await this.quickinput.waitForQuickInputOpened();
await this.quickinput.selectQuickInputElementContaining(kernelGroup);
await this.quickinput.selectQuickInputElementContaining(desiredKernel);
await this.quickinput.waitForQuickInputClosed();
}
await test.step(`Select notebook interpreter: ${desiredKernel}`, async () => {
await expect(this.notebookProgressBar).not.toBeVisible({ timeout: 30000 });
await expect(this.code.driver.page.locator(DETECTING_KERNELS_TEXT)).not.toBeVisible({ timeout: 30000 });

// Wait for the desired kernel to populate in dropdown, if no show then wait for "Select Kernel"
const kernelLabelLocator = this.code.driver.page.locator(KERNEL_LABEL);
try {
await expect(kernelLabelLocator).toHaveText(new RegExp(desiredKernel), { timeout: 10000 });
} catch (e) {
await expect(kernelLabelLocator).toHaveText(new RegExp(SELECT_KERNEL_TEXT), { timeout: 10000 });
}

// Retrieve the matched text for conditional logic
const matchedText = await kernelLabelLocator.textContent() || '';

this.code.logger.log(`Matched text: ${matchedText}, Desired kernel: ${desiredKernel}`);
if (!new RegExp(desiredKernel).test(matchedText)) {
await this.code.driver.page.locator(KERNEL_ACTION).click();
await this.quickinput.waitForQuickInputOpened();
await this.quickinput.selectQuickInputElementContaining(kernelGroup);
await this.quickinput.selectQuickInputElementContaining(desiredKernel);
await this.quickinput.waitForQuickInputClosed();
}

// wait for the kernel to load
await expect(this.code.driver.page.locator('.kernel-action-view-item').locator('.codicon-modifier-spin')).not.toBeVisible({ timeout: 30000 });
});
}

async createNewNotebook() {
Expand All @@ -63,22 +72,28 @@ export class Notebooks {

// Opens a Notebook that lives in the current workspace
async openNotebook(path: string) {
await this.quickaccess.openFileQuickAccessAndWait(basename(path), 1);
await this.quickinput.selectQuickInputElement(0);
await test.step(`Open notebook: ${path}`, async () => {
await this.quickaccess.openFileQuickAccessAndWait(basename(path), 1);
await this.quickinput.selectQuickInputElement(0);

await expect(this.code.driver.page.locator(ACTIVE_ROW_SELECTOR)).toBeVisible();
await this.focusFirstCell();
await expect(this.code.driver.page.locator(ACTIVE_ROW_SELECTOR)).toBeVisible();
await this.focusFirstCell();
});
}

async addCodeToFirstCell(code: string) {
await this.code.driver.page.locator(CELL_LINE).first().click();
await this.waitForTypeInEditor(code);
await this.waitForActiveCellEditorContents(code);
await test.step('Add code to first cell', async () => {
await this.code.driver.page.locator(CELL_LINE).first().click();
await this.typeInEditor(code);
await this.waitForActiveCellEditorContents(code);
});
}

async executeCodeInCell() {
await this.quickaccess.runCommand(EXECUTE_CELL_COMMAND);
await expect(this.code.driver.page.locator(EXECUTE_CELL_SPINNER), 'execute cell spinner to not be visible').not.toBeVisible({ timeout: 30000 });
await test.step('Execute code in cell', async () => {
await this.quickaccess.runCommand(EXECUTE_CELL_COMMAND);
await expect(this.code.driver.page.locator(EXECUTE_CELL_SPINNER), 'execute cell spinner to not be visible').not.toBeVisible({ timeout: 30000 });
});
}

async assertCellOutput(text: string): Promise<void> {
Expand All @@ -96,27 +111,31 @@ export class Notebooks {
}

async runAllCells(timeout: number = 30000) {
await this.code.driver.page.getByLabel('Run All').click();
const stopExecutionLocator = this.code.driver.page.locator('a').filter({ hasText: /Stop Execution|Interrupt/ });
await expect(stopExecutionLocator).toBeVisible();
await expect(stopExecutionLocator).not.toBeVisible({ timeout: timeout });
await test.step('Run all cells', async () => {
await this.code.driver.page.getByLabel('Run All').click();
const stopExecutionLocator = this.code.driver.page.locator('a').filter({ hasText: /Stop Execution|Interrupt/ });
await expect(stopExecutionLocator).toBeVisible();
await expect(stopExecutionLocator).not.toBeVisible({ timeout: timeout });
});
}

async focusFirstCell() {
await this.quickaccess.runCommand('notebook.focusTop');
}

async waitForTypeInEditor(text: string): Promise<any> {
const editor = `${ACTIVE_ROW_SELECTOR} .monaco-editor`;
async typeInEditor(text: string): Promise<any> {
await test.step(`Type in editor: ${text}`, async () => {
const editor = `${ACTIVE_ROW_SELECTOR} .monaco-editor`;

await this.code.driver.page.locator(editor).isVisible();
await this.code.driver.page.locator(editor).isVisible();

const textarea = `${editor} textarea`;
await expect(this.code.driver.page.locator(textarea)).toBeFocused();
const textarea = `${editor} textarea`;
await expect(this.code.driver.page.locator(textarea)).toBeFocused();

await this.code.driver.page.locator(textarea).fill(text);
await this.code.driver.page.locator(textarea).fill(text);

await this._waitForActiveCellEditorContents(c => c.indexOf(text) > -1);
await this._waitForActiveCellEditorContents(c => c.indexOf(text) > -1);
});
}

private async _waitForActiveCellEditorContents(accept: (contents: string) => boolean): Promise<string> {
Expand Down
93 changes: 50 additions & 43 deletions test/e2e/pages/quickaccess.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { Code } from '../infra/code';
import { basename, isAbsolute } from 'path';
import { QuickInput } from './quickInput';
import { expect } from '@playwright/test';
import test, { expect } from '@playwright/test';
import { Editors } from './editors';

enum QuickAccessKind {
Expand Down Expand Up @@ -92,25 +92,27 @@ export class QuickAccess {
}

async openFile(path: string): Promise<void> {
if (!isAbsolute(path)) {
// we require absolute paths to get a single
// result back that is unique and avoid hitting
// the search process to reduce chances of
// search needing longer.
throw new Error('QuickAccess.openFile requires an absolute path');
}
await test.step('Open file', async () => {
if (!isAbsolute(path)) {
// we require absolute paths to get a single
// result back that is unique and avoid hitting
// the search process to reduce chances of
// search needing longer.
throw new Error('QuickAccess.openFile requires an absolute path');
}

const fileName = basename(path);
const fileName = basename(path);

// quick access shows files with the basename of the path
await this.openFileQuickAccessAndWait(path, basename(path));
// quick access shows files with the basename of the path
await this.openFileQuickAccessAndWait(path, basename(path));

// open first element
await this.quickInput.selectQuickInputElement(0);
// open first element
await this.quickInput.selectQuickInputElement(0);

// wait for editor being focused
await this.editors.waitForActiveTab(fileName);
await this.editors.selectTab(fileName);
// wait for editor being focused
await this.editors.waitForActiveTab(fileName);
await this.editors.selectTab(fileName);
});
}

private async openQuickAccessWithRetry(kind: QuickAccessKind, value?: string): Promise<void> {
Expand Down Expand Up @@ -151,39 +153,44 @@ export class QuickAccess {


async runCommand(commandId: string, options?: { keepOpen?: boolean; exactLabelMatch?: boolean }): Promise<void> {
const keepOpen = options?.keepOpen;
const exactLabelMatch = options?.exactLabelMatch;
const stepWrapper = (label: string, fn: () => Promise<void>) => {
try {
// Check if running in a test context
if (test.info().title) {
return test.step(label, fn); // Use test.step if inside a test
}
} catch (e) {
// Catch errors if not in a test context
}
return fn(); // Run directly if not in a test
};

const openCommandPalletteAndTypeCommand = async (): Promise<boolean> => {
// open commands picker
await this.openQuickAccessWithRetry(QuickAccessKind.Commands, `>${commandId}`);
await stepWrapper(`Run command: ${commandId}`, async () => {
const keepOpen = options?.keepOpen;
const exactLabelMatch = options?.exactLabelMatch;

// wait for quick input element text
const text = await this.quickInput.waitForQuickInputElementText();
const openCommandPalletteAndTypeCommand = async (): Promise<boolean> => {
await this.openQuickAccessWithRetry(QuickAccessKind.Commands, `>${commandId}`);
const text = await this.quickInput.waitForQuickInputElementText();

if (text === 'No matching commands' || (exactLabelMatch && text !== commandId)) {
return false;
}
return !(text === 'No matching commands' || (exactLabelMatch && text !== commandId));
};

return true;
};
await expect(async () => {
const hasCommandFound = await openCommandPalletteAndTypeCommand();
if (!hasCommandFound) {
this.code.logger.log(`QuickAccess: No matching commands, retrying...`);
await this.quickInput.closeQuickInput();
throw new Error('Command not found');
}
}).toPass({
timeout: 15000,
intervals: [1000],
});

await expect(async () => {
const hasCommandFound = await openCommandPalletteAndTypeCommand();
if (!hasCommandFound) {
this.code.logger.log(`QuickAccess: No matching commands, retrying...`);
await this.quickInput.closeQuickInput();
throw new Error('Command not found'); // Signal to retry
}
}).toPass({
timeout: 15000,
intervals: [1000],
this.code.logger.log('QuickAccess: Command found and successfully executed.');
await this.quickInput.selectQuickInputElement(0, keepOpen);
});

this.code.logger.log('QuickAccess: Command found and successfully executed.');

// wait and click on best choice
await this.quickInput.selectQuickInputElement(0, keepOpen);
}

async openQuickOutline({ timeout = 30000 }): Promise<void> {
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/tests/notebook/notebook-create.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ test.describe('Notebooks', {
const randomText = Math.random().toString(36).substring(7);

await app.workbench.notebooks.insertNotebookCell('markdown');
await app.workbench.notebooks.waitForTypeInEditor(`## ${randomText} `);
await app.workbench.notebooks.typeInEditor(`## ${randomText} `);
await app.workbench.notebooks.stopEditingCell();
await app.workbench.notebooks.assertMarkdownText('h2', randomText);
});
Expand All @@ -60,7 +60,7 @@ test.describe('Notebooks', {
const randomText = Math.random().toString(36).substring(7);

await app.workbench.notebooks.insertNotebookCell('markdown');
await app.workbench.notebooks.waitForTypeInEditor(`## ${randomText} `);
await app.workbench.notebooks.typeInEditor(`## ${randomText} `);
await app.workbench.notebooks.stopEditingCell();
await app.workbench.notebooks.assertMarkdownText('h2', randomText);
});
Expand Down
Loading