Skip to content

Tests for resources #360

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+340 −5
Open

Tests for resources #360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions src/components/ControlPlane/ActionsMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export type ActionsMenuProps<T> = {
buttonIcon?: string;
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The buttonIcon parameter is still defined in the ActionsMenuProps type (line 17) but has been removed from the function destructuring. This creates an inconsistency where the type declares a parameter that is no longer used. The buttonIcon property should be removed from the ActionsMenuProps type definition to match the implementation.

Suggested change
buttonIcon?: string;

Copilot uses AI. Check for mistakes.
};

export function ActionsMenu<T>({ item, actions, buttonIcon = 'overflow' }: ActionsMenuProps<T>) {
export function ActionsMenu<T>({ item, actions }: ActionsMenuProps<T>) {
const popoverRef = useRef<MenuDomRef>(null);
const [open, setOpen] = useState(false);

Expand All @@ -30,10 +30,11 @@ export function ActionsMenu<T>({ item, actions, buttonIcon = 'overflow' }: Actio

return (
<>
<Button icon={buttonIcon} design="Transparent" onClick={handleOpenerClick} />
<Button icon="overflow" design="Transparent" data-testid="ActionsMenu-opener" onClick={handleOpenerClick} />
<Menu
ref={popoverRef}
open={open}
data-component-name="ActionsMenu"
Copy link
Contributor

@andreaskienle andreaskienle Nov 6, 2025

Choose a reason for hiding this comment

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

Can we use data-testid? This is pretty much standard convention

onItemClick={(event) => {
const element = event.detail.item as HTMLElement & { disabled?: boolean };
const actionKey = element.dataset.actionKey;
Expand Down
328 changes: 328 additions & 0 deletions src/components/ControlPlane/ManagedResources.cy.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,328 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { ManagedResources } from './ManagedResources.tsx';
import { SplitterProvider } from '../Splitter/SplitterContext.tsx';
import { ManagedResourceGroup } from '../../lib/shared/types.ts';
import { MemoryRouter } from 'react-router-dom';
import { useApiResourceMutation } from '../../lib/api/useApiResource.ts';
import '@ui5/webcomponents-cypress-commands';
import { useHandleResourcePatch } from '../../lib/api/types/crossplane/useHandleResourcePatch.ts';

describe('ManagedResources - Delete Resource', () => {
let deleteCalled = false;
let patchCalled = false;
let triggerCallCount = 0;

const fakeUseApiResourceMutation: typeof useApiResourceMutation = (): any => {
return {
data: undefined,
error: undefined,
reset: () => {},
trigger: async (): Promise<any> => {
const currentTriggerCall = triggerCallCount++;
if (currentTriggerCall === 0) {
deleteCalled = true;
} else {
patchCalled = true;
}
return undefined;
},
isMutating: false,
};
};

const mockManagedResources: ManagedResourceGroup[] = [
{
items: [
{
apiVersion: 'account.btp.sap.crossplane.io/v1alpha1',
kind: 'Subaccount',
metadata: {
name: 'test-subaccount',
namespace: 'test-namespace',
creationTimestamp: '2024-01-01T00:00:00Z',
resourceVersion: '1',
labels: {},
},
spec: {},
status: {
conditions: [
{
type: 'Ready',
status: 'True',
lastTransitionTime: '2024-01-01T00:00:00Z',
},
{
type: 'Synced',
status: 'True',
lastTransitionTime: '2024-01-01T00:00:00Z',
},
],
},
} as any,
],
},
];

before(() => {
// Set up interceptors once for all tests
cy.intercept('GET', '**/managed', {
Copy link
Contributor

@andreaskienle andreaskienle Nov 6, 2025

Choose a reason for hiding this comment

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

Any reason for switching to cy.intercept instead of stubbing the hook? By intercepting we couple the component test directly to the HTTP requrst. When we switch to GraphQL later, we will have to rewrite all these tests. By stubbing the hook, ideally, nothing will change for the using component.

statusCode: 200,
body: mockManagedResources,
}).as('getManagedResources');

cy.intercept('GET', '**/customresourcedefinitions*', {
statusCode: 200,
body: {
items: [
{
spec: {
names: {
kind: 'Subaccount',
plural: 'subaccounts',
},
},
},
],
},
}).as('getCRDs');
});

beforeEach(() => {
deleteCalled = false;
patchCalled = false;
triggerCallCount = 0;
});

it('deletes a managed resource', () => {
cy.mount(
<MemoryRouter>
<SplitterProvider>
<ManagedResources useApiResourceMutation={fakeUseApiResourceMutation} />
</SplitterProvider>
</MemoryRouter>,
);

cy.wait('@getManagedResources');
cy.wait('@getCRDs');

cy.get('button[aria-label*="xpand"]').first().click({ force: true });
cy.wait(500);
Copy link
Contributor

@andreaskienle andreaskienle Nov 6, 2025

Choose a reason for hiding this comment

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

Are all these wait calls really necessary? When we add more tests, this will not scale very well. cy.get automatically waits and retries a little bit until the condition is true.

There is a section in the cypress documentation about this: https://docs.cypress.io/app/core-concepts/best-practices#Unnecessary-Waiting

Please check if this is needed.


cy.contains('test-subaccount').should('be.visible');
cy.get('[data-testid="ActionsMenu-opener"]').first().click({ force: true });
cy.contains('Delete').click({ force: true });
cy.get('ui5-dialog[open]').find('ui5-input').typeIntoUi5Input('test-subaccount');

cy.then(() => cy.wrap(deleteCalled).should('equal', false));
cy.get('ui5-dialog[open]').find('ui5-button').contains('Delete').click();
cy.then(() => cy.wrap(deleteCalled).should('equal', true));
});

it('force deletes a managed resource', () => {
cy.mount(
<MemoryRouter>
<SplitterProvider>
<ManagedResources useApiResourceMutation={fakeUseApiResourceMutation} />
</SplitterProvider>
</MemoryRouter>,
);

cy.wait(1000);
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

Use cy.wait('@getManagedResources') and cy.wait('@getCRDs') instead of a hardcoded wait time. This makes the test more reliable and consistent with the other tests in this suite (lines 105-106 and 195-196). Hardcoded waits can lead to flaky tests and unnecessary delays.

Suggested change
cy.wait(1000);
cy.wait('@getManagedResources');
cy.wait('@getCRDs');

Copilot uses AI. Check for mistakes.

cy.get('button[aria-label*="xpand"]').first().click({ force: true });
cy.wait(500);

cy.contains('test-subaccount').should('be.visible');
cy.get('[data-testid="ActionsMenu-opener"]').first().click({ force: true });
cy.contains('Delete').click({ force: true });

// Expand Advanced section
cy.contains('Advanced').click();
cy.wait(500);

// Click directly on "Force deletion" text - this should toggle the checkbox
cy.contains('Force deletion').click({ force: true });
cy.wait(500);

cy.get('ui5-dialog[open]').find('ui5-input').typeIntoUi5Input('test-subaccount');

cy.then(() => cy.wrap(deleteCalled).should('equal', false));
cy.then(() => cy.wrap(patchCalled).should('equal', false));

cy.get('ui5-dialog[open]').find('ui5-button').contains('Delete').click();

cy.wait(2000);

cy.then(() => {
cy.log(`deleteCalled: ${deleteCalled}, patchCalled: ${patchCalled}`);
});

cy.then(() => cy.wrap(deleteCalled).should('equal', true));
cy.then(() => cy.wrap(patchCalled).should('equal', true));
Comment on lines +154 to +161
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

Consider reducing or removing the hardcoded waits (cy.wait(500), cy.wait(2000)) throughout the test file. Cypress generally handles element availability automatically with built-in retry logic. If waits are necessary, consider using more specific assertions or cy.waitUntil() to wait for specific conditions rather than arbitrary time periods, which can make tests slower and more brittle.

Suggested change
cy.wait(2000);
cy.then(() => {
cy.log(`deleteCalled: ${deleteCalled}, patchCalled: ${patchCalled}`);
});
cy.then(() => cy.wrap(deleteCalled).should('equal', true));
cy.then(() => cy.wrap(patchCalled).should('equal', true));
cy.then(() => {
cy.log(`deleteCalled: ${deleteCalled}, patchCalled: ${patchCalled}`);
});
cy.wrap(() => deleteCalled).should('equal', true);
cy.wrap(() => patchCalled).should('equal', true);

Copilot uses AI. Check for mistakes.
});

it('keeps delete button disabled until confirmation text is entered', () => {
// Setup interceptors for this test
cy.intercept('GET', '**/managed', {
statusCode: 200,
body: mockManagedResources,
}).as('getManagedResourcesValidation');

cy.intercept('GET', '**/customresourcedefinitions*', {
statusCode: 200,
body: {
items: [
{
spec: {
names: {
kind: 'Subaccount',
plural: 'subaccounts',
},
},
},
],
},
}).as('getCRDsValidation');

cy.mount(
<MemoryRouter>
<SplitterProvider>
<ManagedResources useApiResourceMutation={fakeUseApiResourceMutation} />
</SplitterProvider>
</MemoryRouter>,
);

cy.wait('@getManagedResourcesValidation');
cy.wait('@getCRDsValidation');

cy.get('button[aria-label*="xpand"]').first().click({ force: true });
cy.wait(500);

cy.contains('test-subaccount').should('be.visible');
cy.get('[data-testid="ActionsMenu-opener"]').first().click({ force: true });
cy.contains('Delete').click({ force: true });

// Delete button should be disabled initially
cy.get('ui5-dialog[open]').find('ui5-button').contains('Delete').should('have.attr', 'disabled');

// Type wrong text
cy.get('ui5-dialog[open]').find('ui5-input').typeIntoUi5Input('wrong-text');
cy.wait(300);

// Delete button should still be disabled
cy.get('ui5-dialog[open]').find('ui5-button').contains('Delete').should('have.attr', 'disabled');

// Clear input by selecting all and deleting
cy.get('ui5-dialog[open]').find('ui5-input').find('input[id*="inner"]').clear({ force: true });
cy.wait(300);

// Type correct text
cy.get('ui5-dialog[open]').find('ui5-input').typeIntoUi5Input('test-subaccount');
cy.wait(300);

// Delete button should now be enabled
cy.get('ui5-dialog[open]').find('ui5-button').contains('Delete').should('not.have.attr', 'disabled');
});
});

describe('ManagedResources - Edit Resource', () => {
let patchHandlerCreated = false;

const fakeUseHandleResourcePatch: typeof useHandleResourcePatch = () => {
patchHandlerCreated = true;
return async () => {
return true;
};
};

const mockManagedResources: ManagedResourceGroup[] = [
{
items: [
{
apiVersion: 'account.btp.sap.crossplane.io/v1alpha1',
kind: 'Subaccount',
metadata: {
name: 'test-subaccount',
namespace: 'test-namespace',
creationTimestamp: '2024-01-01T00:00:00Z',
resourceVersion: '1',
labels: {},
},
spec: {},
status: {
conditions: [
{
type: 'Ready',
status: 'True',
lastTransitionTime: '2024-01-01T00:00:00Z',
},
{
type: 'Synced',
status: 'True',
lastTransitionTime: '2024-01-01T00:00:00Z',
},
],
},
} as any,
],
},
];

before(() => {
cy.intercept('GET', '**/managed', {
statusCode: 200,
body: mockManagedResources,
}).as('getManagedResources');

cy.intercept('GET', '**/customresourcedefinitions*', {
statusCode: 200,
body: {
items: [
{
spec: {
names: {
kind: 'Subaccount',
plural: 'subaccounts',
},
},
},
],
},
}).as('getCRDs');
});

beforeEach(() => {
patchHandlerCreated = false;
});

it('initializes patch handler and edit button is available', () => {
cy.mount(
<MemoryRouter>
<SplitterProvider>
<ManagedResources useHandleResourcePatch={fakeUseHandleResourcePatch} />
</SplitterProvider>
</MemoryRouter>,
);

cy.wait('@getManagedResources');
cy.wait('@getCRDs');

// Verify patch handler was initialized
cy.then(() => cy.wrap(patchHandlerCreated).should('equal', true));

cy.get('button[aria-label*="xpand"]').first().click({ force: true });
cy.wait(500);

cy.contains('test-subaccount').should('be.visible');
cy.get('[data-testid="ActionsMenu-opener"]').first().click({ force: true });

// Verify Edit button exists
cy.contains('Edit').should('exist');

// Verify Edit button is not disabled (check separately)
cy.contains('Edit').should('not.have.attr', 'disabled');

// Click Edit button
cy.contains('Edit').click({ force: true });
});
});
12 changes: 9 additions & 3 deletions src/components/ControlPlane/ManagedResources.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
Toolbar,
ToolbarSpacer,
} from '@ui5/webcomponents-react';
import { useApiResource, useApiResourceMutation } from '../../lib/api/useApiResource';
import { useApiResource, useApiResourceMutation as _useApiResourceMutation } from '../../lib/api/useApiResource';
import { ManagedResourcesRequest } from '../../lib/api/types/crossplane/listManagedResources';
import { formatDateAsTimeAgo } from '../../utils/i18n/timeAgo';
import IllustratedError from '../Shared/IllustratedError';
Expand All @@ -34,7 +34,7 @@ import { useSplitter } from '../Splitter/SplitterContext.tsx';
import { YamlSidePanel } from '../Yaml/YamlSidePanel.tsx';
import { ErrorDialog, ErrorDialogHandle } from '../Shared/ErrorMessageBox.tsx';
import { APIError } from '../../lib/api/error.ts';
import { useHandleResourcePatch } from '../../lib/api/types/crossplane/useHandleResourcePatch.ts';
import { useHandleResourcePatch as _useHandleResourcePatch } from '../../lib/api/types/crossplane/useHandleResourcePatch.ts';

interface StatusFilterColumn {
filterValue?: string;
Expand All @@ -54,7 +54,13 @@ type ResourceRow = {
conditionSyncedMessage: string;
};

export function ManagedResources() {
export function ManagedResources({
useApiResourceMutation = _useApiResourceMutation,
useHandleResourcePatch = _useHandleResourcePatch,
}: {
useApiResourceMutation?: typeof _useApiResourceMutation;
useHandleResourcePatch?: typeof _useHandleResourcePatch;
} = {}) {
const { t } = useTranslation();
const toast = useToast();
const { openInAside } = useSplitter();
Expand Down
Loading