Skip to content
Draft
Changes from all 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
128 changes: 102 additions & 26 deletions lumigator/frontend/e2e/datasets.spec.ts
Original file line number Diff line number Diff line change
@@ -1,47 +1,123 @@
import { test, expect } from '@playwright/test'
import path from 'path'
import { log } from 'console';

Check failure on line 2 in lumigator/frontend/e2e/datasets.spec.ts

View workflow job for this annotation

GitHub Actions / Lint, Format, Type check (Frontend)

'log' is defined but never used
import path from 'path';
import fs from 'fs';

// get the sample dataset absolute file path
const currentDir = path.dirname(new URL(import.meta.url).pathname)
const currentDir = path.dirname(new URL(import.meta.url).pathname);
const sampleDatasetFilePath = path.resolve(
currentDir,
'../../sample_data/summarization/dialogsum_exc.csv',
)
'../../sample_data/summarization/dialogsum_mini_no_gt.csv'
);

test('successfully uploads a dataset', async ({ page }) => {
await page.goto('/')
// Create a unique file name by appending a timestamp.
const timestamp = Date.now();
const dynamicFileName = `dialogsum_mini_no_gt_${timestamp}.csv`;
let submittedFileName = dynamicFileName;
Comment on lines +15 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: no need for a different variable


// find & click the provide dataset button and capture the system file chooser dialog
const fileChooserPromise = page.waitForEvent('filechooser')
const button = page.getByRole('button', { name: 'Provide Dataset' })
await button.click()
const fileChooser = await fileChooserPromise

// choose the sample dataset file from the file chooser dialog
await fileChooser.setFiles(sampleDatasetFilePath)
test('Launch a GT workflow with unique file and fail early on job failure', async ({ page }) => {
// Increase test timeout to 10 minutes.
test.setTimeout(600000);
Copy link
Contributor

@khaledosman khaledosman Apr 17, 2025

Choose a reason for hiding this comment

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

is this really needed? having a test run for 10 minutes likely means that other tests can't be run until its finished (unless we explicitly tell playwright to run in parallel), and it would be too slow for a CI feedback, maybe its ok to rely on unit/integration tests only for the actual job stuff?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this because the default timeout is 3 mins (I think) and this test takes usually something like 7 minutes. When I wrote this I was thinking to run this in parallel to other jobs, what do you think?

Copy link
Contributor

@khaledosman khaledosman Apr 22, 2025

Choose a reason for hiding this comment

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

hmm I think by default cypress runs the tests sequentially, although it can be configured to run in parallel, but idk even in parallel if it splits them across 4 cores or so, 7 mins per test still sounds a bit too slow, I'm not sure if its worth it.. especially considering jobs can fail for all kinds of reasons and there's alot of combination of inputs/models to test

await page.goto('/');

// click the upload button from the confirmation modal
const uploadButton = page.getByRole('button', { name: 'Upload' })
await uploadButton.click()
// Trigger file chooser by clicking "Provide Dataset".
const fileChooserPromise = page.waitForEvent('filechooser');
const provideDatasetButton = page.getByRole('button', { name: 'Provide Dataset' });
await provideDatasetButton.click();
const fileChooser = await fileChooserPromise;

// wait for the api requests to upload the file and refetch the datasets
const fileBuffer = fs.readFileSync(sampleDatasetFilePath);
await fileChooser.setFiles({
name: dynamicFileName,
mimeType: 'text/csv',
buffer: fileBuffer,
});

// Click the "Upload" button from the confirmation modal.
const uploadButton = page.getByRole('button', { name: 'Upload' });
await uploadButton.click();

// Wait for the API requests for upload and refresh.
const [response] = await Promise.all([
page.waitForResponse(
(response) =>
response.url().includes('datasets') &&
response.status() === 201 &&
response.request().method() === 'POST',
response.request().method() === 'POST'
),
page.waitForRequest(
(request) => request.url().includes('datasets') && request.method() === 'GET',
(request) => request.url().includes('datasets') && request.method() === 'GET'
),
])
]);

// get the returned filename and id from the api response
const { id, filename } = await response.json()
// Extract returned dataset id and filename from the API response.
const { id, filename } = await response.json();

// wait for the new table row with the new dataset id and filename to be rendered, (ids are shortened so we only check the first 20 characters)
// Wait for the new dataset row to appear in the table.
await expect(
page.locator(`table tr:has(td:text("${id.slice(0, 20)}")):has(td:text("${filename}"))`),
).toBeVisible()
})
page.locator(`table tr:has(td:text("${id.slice(0, 20)}")):has(td:text("${filename}"))`)
).toBeVisible();

// Click the dataset row (using the unique submittedFileName).
const datasetRow = page.locator('tr').filter({ hasText: submittedFileName }).first();
await expect(datasetRow).toBeVisible({ timeout: 5000 });
Copy link
Contributor

@khaledosman khaledosman Apr 17, 2025

Choose a reason for hiding this comment

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

is the timeout necessary here and the other toBeVisible calls? I think the default timeout/behavior should be fine

await datasetRow.click();

// Wait for the sidebar (with class "sliding-panel") to appear.
const sidebar = page.locator('.sliding-panel');
await expect(sidebar).toBeVisible({ timeout: 5000 });

// In the sidebar, click the "Generate Ground Truth" button.
const generateGtButton = sidebar.getByRole('button', { name: 'Generate Ground Truth' });
await expect(generateGtButton).toBeVisible({ timeout: 5000 });
await generateGtButton.click();

// Wait for the popup to appear (identified by its ".popup" class).
const popupContainer = page.locator('.popup');
await expect(popupContainer).toBeVisible({ timeout: 5000 });

// Click "Start Generating" and wait for the API to create the job.
const [jobResponse] = await Promise.all([
page.waitForResponse(
(res) =>
res.url().includes('/jobs') &&
res.request().method() === 'POST' &&
res.status() === 201
),
popupContainer.getByRole('button', { name: 'Start Generating' }).click(),
]);
Comment on lines +81 to +89
Copy link
Contributor

@khaledosman khaledosman Apr 17, 2025

Choose a reason for hiding this comment

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

nit: I don't necessarily think we need to wait for both in parallel since one is a consequence of the other

Suggested change
const [jobResponse] = await Promise.all([
page.waitForResponse(
(res) =>
res.url().includes('/jobs') &&
res.request().method() === 'POST' &&
res.status() === 201
),
popupContainer.getByRole('button', { name: 'Start Generating' }).click(),
]);
await popupContainer.getByRole('button', { name: 'Start Generating' }).click()
const jobResponse = await page.waitForResponse(
(res) =>
res.url().includes('/jobs') &&
res.request().method() === 'POST' &&
res.status() === 201
)


// Extract the job ID from the API response.
const jobData = await jobResponse.json();
const jobId = jobData.id; // e.g., "712afa3f-ee81-41c0-8f1b-8939c2bf0ee4"

Check failure on line 93 in lumigator/frontend/e2e/datasets.spec.ts

View workflow job for this annotation

GitHub Actions / Lint, Format, Type check (Frontend)

'jobId' is assigned a value but never used

// Switch to the "Ground Truth Jobs" tab.
const groundTruthJobsTab = page.locator('button').filter({ hasText: 'Ground Truth Jobs' });
await groundTruthJobsTab.click();


await page.waitForFunction(
(fileName) => {
const rows = Array.from(document.querySelectorAll('tr')).filter(row =>
(row.textContent || "").includes(fileName)
);
if (rows.length === 0) return false;
const lastRow = rows[rows.length - 1];
const statusEl = lastRow.querySelector('.p-tag-label');
if (!statusEl || !(statusEl instanceof HTMLElement)) return false;
const status = statusEl.innerText.trim().toLowerCase();
if (status === 'failed') {
throw new Error('Job failed');
}
return status === 'succeeded';
},
submittedFileName,
{ timeout: 600000 }
);

// Assert that the last row’s status is "succeeded".
const jobRows = page.locator(`tr:has(td:has-text("${submittedFileName}"))`);
const lastJobRow = jobRows.last();
await expect(lastJobRow.locator('.p-tag-label')).toHaveText(/succeeded/i);
});
Loading