Skip to content
Merged
Show file tree
Hide file tree
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
8 changes: 8 additions & 0 deletions packages/cypress/cypress/pages/appChrome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ class AppChrome {
findNavItem(args: { name: string; rootSection?: string; subSection?: string }) {
return this.findSideBar().findAppNavItem(args);
}

findDarkThemeToggle() {
return cy.findByTestId('dark-theme-toggle');
}

findLightThemeToggle() {
return cy.findByTestId('light-theme-toggle');
}
}

export const appChrome = new AppChrome();
12 changes: 0 additions & 12 deletions packages/cypress/cypress/pages/mlflowExperiments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,24 +204,12 @@ class MlflowExperiments {
return cy.contains('Metrics', { timeout: 10000 });
}

findDarkThemeToggle() {
return cy.findByTestId('dark-theme-toggle');
}

findLightThemeToggle() {
return cy.findByTestId('light-theme-toggle');
}

getMlflowDarkModeStorageValue(): Cypress.Chainable<string | null> {
return cy.window().then((win) => {
const value = win.localStorage.getItem(MLFLOW_DARK_MODE_KEY);
return cy.wrap<string | null>(value);
});
}

getHtmlDarkModeClass(): Cypress.Chainable<boolean> {
return cy.document().then((doc) => doc.documentElement.classList.contains('pf-v6-theme-dark'));
}
}

export const mlflowExperiments = new MlflowExperiments();
26 changes: 13 additions & 13 deletions packages/cypress/cypress/pages/promptManagement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,19 @@ class PromptManagement {
}

findProjectSelector() {
return cy.findByTestId('project-selector-dropdown');
return cy.findByTestId('project-selector-toggle', { timeout: 30000 });
}

findProjectInDropdown(name: string) {
return cy.findByRole('menuitem', { name });
}

shouldHaveWorkspace(workspace: string) {
cy.url().should('include', `workspace=${workspace}`);
}

findErrorEmptyState() {
return cy.findByTestId('empty-state-title', { timeout: 10000 });
}

findPromptsSearchInput() {
Expand Down Expand Up @@ -93,24 +105,12 @@ class PromptManagement {
return cy.findByRole('radio', { name: 'Preview' });
}

findDarkThemeToggle() {
return cy.findByTestId('dark-theme-toggle');
}

findLightThemeToggle() {
return cy.findByTestId('light-theme-toggle');
}

getMlflowDarkModeStorageValue(): Cypress.Chainable<string | null> {
return cy.window().then((win) => {
const value = win.localStorage.getItem(MLFLOW_DARK_MODE_KEY);
return cy.wrap<string | null>(value);
});
}

getHtmlDarkModeClass(): Cypress.Chainable<boolean> {
return cy.document().then((doc) => doc.documentElement.classList.contains('pf-v6-theme-dark'));
}
}

export const promptManagement = new PromptManagement();
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe('Verify Prompt Management page', () => {
it(
'Create a prompt and verify it appears in the prompts table',
{
tags: ['@Sanity', '@SanitySet1', '@PromptManagement', '@MLflow'],
tags: ['@Sanity', '@SanitySet1', '@PromptManagement', '@MLflow', '@NonConcurrent'],
},
() => {
const prompt = testData.prompts[0];
Expand Down Expand Up @@ -145,20 +145,6 @@ describe('Verify Prompt Management page', () => {

cy.step('Verify the prompt persists after navigation');
promptManagement.findPromptInTable(prompt.name).should('be.visible');

cy.step('Toggle dark mode on');
promptManagement.findDarkThemeToggle().click();

cy.step('Verify dark theme is applied');
promptManagement.getHtmlDarkModeClass().should('equal', true);
promptManagement.getMlflowDarkModeStorageValue().should('equal', 'true');

cy.step('Toggle light mode back on');
promptManagement.findLightThemeToggle().click();

cy.step('Verify light theme is restored');
promptManagement.getHtmlDarkModeClass().should('equal', false);
promptManagement.getMlflowDarkModeStorageValue().should('equal', 'false');
},
);
});
111 changes: 111 additions & 0 deletions packages/cypress/cypress/tests/mocked/mlflow/mlflowExperiments.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import {
mockDashboardConfig,
mockK8sResourceList,
mockProjectK8sResource,
} from '@odh-dashboard/internal/__mocks__';
import { mockDscStatus } from '@odh-dashboard/internal/__mocks__/mockDscStatus';
import { DataScienceStackComponent } from '@odh-dashboard/internal/concepts/areas/types';
import { ProjectModel } from '../../../utils/models';
import { asProductAdminUser } from '../../../utils/mockUsers';
import { interceptMlflowStatus } from '../../../utils/mlflowUtils';
import { mlflowExperiments } from '../../../pages/mlflowExperiments';
import { appChrome } from '../../../pages/appChrome';

const PROJECT_A = 'test-project-a';
const PROJECT_B = 'test-project-b';

const initIntercepts = ({ mlflowConfigured = true }: { mlflowConfigured?: boolean } = {}) => {
asProductAdminUser();
cy.interceptOdh('GET /api/config', mockDashboardConfig({}));
interceptMlflowStatus(mlflowConfigured);

const projectA = mockProjectK8sResource({ k8sName: PROJECT_A, displayName: PROJECT_A });
const projectB = mockProjectK8sResource({ k8sName: PROJECT_B, displayName: PROJECT_B });
cy.interceptK8sList(ProjectModel, mockK8sResourceList([projectA, projectB]));
cy.interceptK8s(ProjectModel, projectA);
};

describe('MLflow Experiments page wrapper', () => {
beforeEach(() => {
initIntercepts();
});

describe('Page chrome and navigation', () => {
it('should display page title and Launch MLflow button', () => {
mlflowExperiments.visit(PROJECT_A);
mlflowExperiments.findPageTitle().should('be.visible');
mlflowExperiments
.findLaunchMlflowButton()
.should('be.visible')
.should('have.attr', 'href', '/mlflow')
.should('have.attr', 'target', '_blank');
});

it('should navigate via sidebar and show active nav item', () => {
cy.visitWithLogin('/');
appChrome.findMainContent().should('be.visible');

mlflowExperiments.findNavSection().click();
mlflowExperiments.navigate();

mlflowExperiments.shouldHaveExperimentsUrl();
mlflowExperiments.findNavItem().should('have.attr', 'aria-current', 'page');
});
});

describe('Project selector', () => {
it('should switch workspace when selecting a different project', () => {
mlflowExperiments.visit(PROJECT_A);
mlflowExperiments.findProjectSelector().should('contain', PROJECT_A);

mlflowExperiments.findProjectSelector().click();
mlflowExperiments.findProjectInDropdown(PROJECT_B).click();
mlflowExperiments.shouldHaveWorkspace(PROJECT_B);

mlflowExperiments.findProjectSelector().click();
mlflowExperiments.findProjectInDropdown(PROJECT_A).click();
mlflowExperiments.shouldHaveWorkspace(PROJECT_A);
});
});

describe('Dark mode toggle', () => {
it('should sync localStorage on toggle', () => {
mlflowExperiments.visit(PROJECT_A);

appChrome.findDarkThemeToggle().click();
mlflowExperiments.getMlflowDarkModeStorageValue().should('equal', 'true');

appChrome.findLightThemeToggle().click();
mlflowExperiments.getMlflowDarkModeStorageValue().should('equal', 'false');
});
});
Comment on lines +71 to +81
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Same idempotency gap as in promptManagement.cy.ts, plus a misleading title.

  1. The title says "should sync dark mode class and localStorage on toggle" but the body only asserts localStorage. Either drop "class" from the title or also assert the <html> class (e.g. cy.get('html').should('have.class', 'pf-v6-theme-dark')).
  2. localStorage is not cleared between tests, so the first click() may be toggling from an already-dark baseline and the 'true' assertion passes trivially. Add cy.clearLocalStorage() (or assert the pre-toggle baseline) to make the test prove a real round-trip.
🔧 Proposed fix
   describe('Dark mode toggle', () => {
-    it('should sync dark mode class and localStorage on toggle', () => {
+    it('should sync localStorage on toggle', () => {
+      cy.clearLocalStorage();
       mlflowExperiments.visit(PROJECT_A);
+      mlflowExperiments.getMlflowDarkModeStorageValue().should('not.equal', 'true');
 
       appChrome.findDarkThemeToggle().click();
       mlflowExperiments.getMlflowDarkModeStorageValue().should('equal', 'true');
 
       appChrome.findLightThemeToggle().click();
       mlflowExperiments.getMlflowDarkModeStorageValue().should('equal', 'false');
     });
   });

As per coding guidelines: "Tests must be idempotent: runnable in any order without shared state."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cypress/cypress/tests/mocked/mlflow/mlflowExperiments.cy.ts` around
lines 71 - 81, Update the test in the Dark mode toggle spec to be idempotent and
accurate: before interacting, call cy.clearLocalStorage() (or assert a known
baseline via mlflowExperiments.getMlflowDarkModeStorageValue()) so the first
click cannot rely on prior state; change the it title to remove "class" or add
an assertion for the HTML class (e.g. use cy.get('html').should('have.class',
'pf-v6-theme-dark')) alongside the existing localStorage checks; keep the
existing calls to mlflowExperiments.visit(PROJECT_A),
appChrome.findDarkThemeToggle()/findLightThemeToggle(), and
mlflowExperiments.getMlflowDarkModeStorageValue() but add the precondition
clear/assert and the html class assertion if you want the title to remain
unchanged.


describe('Error states', () => {
it('should show error state for invalid workspace', () => {
const invalidWorkspace = 'nonexistent-project';
mlflowExperiments.visit(invalidWorkspace);
mlflowExperiments
.findErrorEmptyState()
.should('be.visible')
.should('contain', invalidWorkspace);
});

it('should show unavailable state when MLflow is not configured', () => {
initIntercepts({ mlflowConfigured: false });
mlflowExperiments.visit(PROJECT_A);
mlflowExperiments.findMlflowUnavailableState().should('be.visible');
});

it('should hide nav item when MLflow operator is removed', () => {
const dscStatus = mockDscStatus({});
dscStatus.components = {
...dscStatus.components,
[DataScienceStackComponent.MLFLOW]: { managementState: 'Removed' },
};
cy.interceptOdh('GET /api/dsc/status', dscStatus);

cy.visitWithLogin('/');
mlflowExperiments.findNavItem().should('not.exist');
});
});
});
109 changes: 109 additions & 0 deletions packages/cypress/cypress/tests/mocked/mlflow/promptManagement.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import {
mockDashboardConfig,
mockK8sResourceList,
mockProjectK8sResource,
} from '@odh-dashboard/internal/__mocks__';
import { mockDscStatus } from '@odh-dashboard/internal/__mocks__/mockDscStatus';
import { DataScienceStackComponent } from '@odh-dashboard/internal/concepts/areas/types';
import { ProjectModel } from '../../../utils/models';
import { asProductAdminUser } from '../../../utils/mockUsers';
import { interceptMlflowStatus } from '../../../utils/mlflowUtils';
import { promptManagement } from '../../../pages/promptManagement';
import { appChrome } from '../../../pages/appChrome';

const PROJECT_A = 'test-project-a';
const PROJECT_B = 'test-project-b';

const initIntercepts = ({
mlflowConfigured = true,
genAiStudio = true,
}: { mlflowConfigured?: boolean; genAiStudio?: boolean } = {}) => {
asProductAdminUser();
cy.interceptOdh('GET /api/config', mockDashboardConfig({ genAiStudio }));
interceptMlflowStatus(mlflowConfigured);

const projectA = mockProjectK8sResource({ k8sName: PROJECT_A, displayName: PROJECT_A });
const projectB = mockProjectK8sResource({ k8sName: PROJECT_B, displayName: PROJECT_B });
cy.interceptK8sList(ProjectModel, mockK8sResourceList([projectA, projectB]));
cy.interceptK8s(ProjectModel, projectA);
};

describe('Prompt Management page wrapper', () => {
beforeEach(() => {
initIntercepts();
});

describe('Page chrome', () => {
it('should display page title and Launch MLflow button', () => {
promptManagement.visit(PROJECT_A);
promptManagement.findPageTitle().should('be.visible');
promptManagement
.findLaunchMlflowButton()
.should('be.visible')
.should('have.attr', 'href', '/mlflow')
.should('have.attr', 'target', '_blank');
});
});

describe('Project selector', () => {
it('should switch workspace when selecting a different project', () => {
promptManagement.visit(PROJECT_A);
promptManagement.findProjectSelector().should('contain', PROJECT_A);

promptManagement.findProjectSelector().click();
promptManagement.findProjectInDropdown(PROJECT_B).click();
promptManagement.shouldHaveWorkspace(PROJECT_B);

promptManagement.findProjectSelector().click();
promptManagement.findProjectInDropdown(PROJECT_A).click();
promptManagement.shouldHaveWorkspace(PROJECT_A);
});
});

describe('Dark mode toggle', () => {
it('should sync localStorage on toggle', () => {
promptManagement.visit(PROJECT_A);

appChrome.findDarkThemeToggle().click();
promptManagement.getMlflowDarkModeStorageValue().should('equal', 'true');

appChrome.findLightThemeToggle().click();
promptManagement.getMlflowDarkModeStorageValue().should('equal', 'false');
});
});
Comment on lines +63 to +73
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Dark-mode test leaks localStorage across specs/tests.

The MLflow dark-mode flag is written to localStorage (key _mlflow_dark_mode_toggle_enabled). Cypress 13 does not automatically clear localStorage between tests, so if any prior test (or a previous run in --no-exit/open mode) left 'true' behind, the click on findDarkThemeToggle() may toggle into an already-dark state and the subsequent assertion should('equal', 'true') becomes a no-op rather than a genuine round-trip check. Worse, the findLightThemeToggle().click() assertion flips brittle in the reverse order.

Make the pre-conditions explicit so the test is idempotent and actually proves the toggle behavior.

🔧 Proposed fix
   describe('Dark mode toggle', () => {
     it('should sync localStorage on toggle', () => {
+      cy.clearLocalStorage();
       promptManagement.visit(PROJECT_A);
 
+      // Baseline: light theme
+      promptManagement.getMlflowDarkModeStorageValue().should('not.equal', 'true');
+
       appChrome.findDarkThemeToggle().click();
       promptManagement.getMlflowDarkModeStorageValue().should('equal', 'true');
 
       appChrome.findLightThemeToggle().click();
       promptManagement.getMlflowDarkModeStorageValue().should('equal', 'false');
     });
   });

As per coding guidelines: "Tests must be idempotent: runnable in any order without shared state."

📝 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
describe('Dark mode toggle', () => {
it('should sync localStorage on toggle', () => {
promptManagement.visit(PROJECT_A);
appChrome.findDarkThemeToggle().click();
promptManagement.getMlflowDarkModeStorageValue().should('equal', 'true');
appChrome.findLightThemeToggle().click();
promptManagement.getMlflowDarkModeStorageValue().should('equal', 'false');
});
});
describe('Dark mode toggle', () => {
it('should sync localStorage on toggle', () => {
cy.clearLocalStorage();
promptManagement.visit(PROJECT_A);
// Baseline: light theme
promptManagement.getMlflowDarkModeStorageValue().should('not.equal', 'true');
appChrome.findDarkThemeToggle().click();
promptManagement.getMlflowDarkModeStorageValue().should('equal', 'true');
appChrome.findLightThemeToggle().click();
promptManagement.getMlflowDarkModeStorageValue().should('equal', 'false');
});
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cypress/cypress/tests/mocked/mlflow/promptManagement.cy.ts` around
lines 46 - 56, Ensure the test sets a deterministic MLflow dark-mode
precondition before exercising the toggles: before calling
promptManagement.visit(PROJECT_A) or before clicking, explicitly set or clear
the localStorage key `_mlflow_dark_mode_toggle_enabled` via promptManagement
helpers (or use promptManagement.clearMlflowDarkModeStorage /
promptManagement.setMlflowDarkModeStorage) so the initial state is known, then
assert the toggle changes the value using
appChrome.findDarkThemeToggle().click(),
promptManagement.getMlflowDarkModeStorageValue(), and
appChrome.findLightThemeToggle().click(); this makes the test idempotent and
removes leakage across specs.


describe('Error states', () => {
it('should show error state for invalid workspace', () => {
const invalidWorkspace = 'nonexistent-project';
promptManagement.visit(invalidWorkspace);
promptManagement
.findErrorEmptyState()
.should('be.visible')
.should('contain', invalidWorkspace);
});

it('should show unavailable state when MLflow is not configured', () => {
initIntercepts({ mlflowConfigured: false });
promptManagement.visit(PROJECT_A);
promptManagement.findMlflowUnavailableState().should('be.visible');
});

it('should hide nav item when genAiStudio feature flag is disabled', () => {
initIntercepts({ genAiStudio: false });
cy.visitWithLogin('/');
promptManagement.findNavItem().should('not.exist');
});

it('should hide nav item when MLflow operator is removed', () => {
const dscStatus = mockDscStatus({});
dscStatus.components = {
...dscStatus.components,
[DataScienceStackComponent.MLFLOW]: { managementState: 'Removed' },
};
cy.interceptOdh('GET /api/dsc/status', dscStatus);

cy.visitWithLogin('/');
promptManagement.findNavItem().should('not.exist');
});
});
});
Loading