Skip to content

HP-2382 Migrate PartCreateCest.php Codeception Test to Playwright and remove Legacy Test #185

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

Merged
Show file tree
Hide file tree
Changes from 2 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
92 changes: 92 additions & 0 deletions tests/playwright/e2e/manager/part-create.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { test } from "@hipanel-core/fixtures";
import {expect} from "@playwright/test";
import PartCreateView from "@hipanel-module-stock/page/PartCreateView";

const partData = {
partno: 'CHASSIS EPYC 7402P',
src_id: 'TEST-DS-01',
dst_id: 'TEST-DS-02',
serials: `MG_TEST_PART${Date.now()}`,
move_descr: 'MG TEST MOVE',
price: 200,
currency: 'usd',
company_id: 'Other',
};
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use dynamic serials for all tests to prevent collisions

The current implementation uses a timestamp for serials, but it's defined once at the top level. This can cause collisions if tests run quickly in sequence.

-const partData = {
+function createPartData() {
+  return {
    partno: 'CHASSIS EPYC 7402P',
    src_id: 'TEST-DS-01',
    dst_id: 'TEST-DS-02',
-    serials: `MG_TEST_PART${Date.now()}`,
+    serials: `MG_TEST_PART${Date.now()}_${Math.floor(Math.random() * 1000)}`,
    move_descr: 'MG TEST MOVE',
    price: 200,
    currency: 'usd',
    company_id: 'Other',
+  };
};
📝 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
const partData = {
partno: 'CHASSIS EPYC 7402P',
src_id: 'TEST-DS-01',
dst_id: 'TEST-DS-02',
serials: `MG_TEST_PART${Date.now()}`,
move_descr: 'MG TEST MOVE',
price: 200,
currency: 'usd',
company_id: 'Other',
};
function createPartData() {
return {
partno: 'CHASSIS EPYC 7402P',
src_id: 'TEST-DS-01',
dst_id: 'TEST-DS-02',
serials: `MG_TEST_PART${Date.now()}_${Math.floor(Math.random() * 1000)}`,
move_descr: 'MG TEST MOVE',
price: 200,
currency: 'usd',
company_id: 'Other',
};
};


test.describe('Part Management', () => {
test('Ensure part management buttons work @hipanel-module-stock @manager', async ({ managerPage }) => {
const partView = new PartCreateView(managerPage);
await partView.navigate();

let n = await managerPage.locator('div.item').count();
expect(n).toBe(1);

await partView.addPart();
expect(await managerPage.locator('div.item').count()).toBe(++n);

await partView.addPart();
expect(await managerPage.locator('div.item').count()).toBe(++n);

await partView.copyPart();
expect(await managerPage.locator('div.item').count()).toBe(++n);

await partView.removePart();
expect(await managerPage.locator('div.item').count()).toBe(--n);

await partView.removePart();
expect(await managerPage.locator('div.item').count()).toBe(--n);

await partView.removePart();
expect(await managerPage.locator('div.item').count()).toBe(--n);
});

test('Ensure part cannot be created without data @hipanel-module-stock @manager', async ({ managerPage }) => {
const partView = new PartCreateView(managerPage);
await partView.navigate();
await partView.save();

const errorMessages = [
'Part No. cannot be blank.',
'Source cannot be blank.',
'Destination cannot be blank.',
'Serials cannot be blank.',
'Move description cannot be blank.',
'Purchase price cannot be blank.',
'Currency cannot be blank.',
];

for (const message of errorMessages) {
await expect(managerPage.locator(`text=${message}`)).toBeVisible();
}
});

test('Ensure a part can be created @hipanel-module-stock @manager', async ({ managerPage }) => {
const partView = new PartCreateView(managerPage);
await partView.navigate();
await partView.fillPartFields(partData);
await partView.save();
await expect(managerPage.locator('text=Part has been created')).toBeVisible();
});
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use dynamic test data for each test

Use the function suggested earlier to create unique test data for each test to prevent conflicts between tests.

test('Ensure a part can be created @hipanel-module-stock @manager', async ({ managerPage }) => {
    const partView = new PartCreateView(managerPage);
+    const uniquePartData = createPartData();
    await partView.navigate();
-    await partView.fillPartFields(partData);
+    await partView.fillPartFields(uniquePartData);
    await partView.save();
    await expect(managerPage.locator('text=Part has been created')).toBeVisible();
});
📝 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
test('Ensure a part can be created @hipanel-module-stock @manager', async ({ managerPage }) => {
const partView = new PartCreateView(managerPage);
await partView.navigate();
await partView.fillPartFields(partData);
await partView.save();
await expect(managerPage.locator('text=Part has been created')).toBeVisible();
});
test('Ensure a part can be created @hipanel-module-stock @manager', async ({ managerPage }) => {
const partView = new PartCreateView(managerPage);
const uniquePartData = createPartData();
await partView.navigate();
await partView.fillPartFields(uniquePartData);
await partView.save();
await expect(managerPage.locator('text=Part has been created')).toBeVisible();
});


test('Ensure multiple parts can be created @hipanel-module-stock @manager', async ({ managerPage }) => {
const partView = new PartCreateView(managerPage);
await partView.navigate();
await partView.fillPartFields(partData);
await partView.addPart();
await partView.save();
await expect(managerPage.locator('text=Part has been created')).toBeVisible();
});
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use dynamic test data and add validation for multiple parts

Each test should use unique data, and this test should verify that multiple parts were actually created.

test('Ensure multiple parts can be created @hipanel-module-stock @manager', async ({ managerPage }) => {
    const partView = new PartCreateView(managerPage);
+    const uniquePartData = createPartData();
    await partView.navigate();
-    await partView.fillPartFields(partData);
+    await partView.fillPartFields(uniquePartData);
    await partView.addPart();
+    // Fill the second part with different data
+    const secondPartData = createPartData();
+    await partView.fillPartFields(secondPartData);
    await partView.save();
    await expect(managerPage.locator('text=Part has been created')).toBeVisible();
+    // Verify that multiple parts were created
+    await expect(managerPage.locator('text=Created 2 parts')).toBeVisible();
});
📝 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
test('Ensure multiple parts can be created @hipanel-module-stock @manager', async ({ managerPage }) => {
const partView = new PartCreateView(managerPage);
await partView.navigate();
await partView.fillPartFields(partData);
await partView.addPart();
await partView.save();
await expect(managerPage.locator('text=Part has been created')).toBeVisible();
});
test('Ensure multiple parts can be created @hipanel-module-stock @manager', async ({ managerPage }) => {
const partView = new PartCreateView(managerPage);
const uniquePartData = createPartData();
await partView.navigate();
await partView.fillPartFields(uniquePartData);
await partView.addPart();
// Fill the second part with different data
const secondPartData = createPartData();
await partView.fillPartFields(secondPartData);
await partView.save();
await expect(managerPage.locator('text=Part has been created')).toBeVisible();
// Verify that multiple parts were created
await expect(managerPage.locator('text=Created 2 parts')).toBeVisible();
});


test('Ensure a part can be created and then deleted @hipanel-module-stock @manager', async ({ managerPage }) => {
const partView = new PartCreateView(managerPage);
await partView.navigate();
await partView.fillPartFields(partData);
await partView.save();
await expect(managerPage.locator('text=Part has been created')).toBeVisible();

await managerPage.click('text=Delete');
managerPage.on('dialog', async dialog => await dialog.accept());
await expect(managerPage.locator('text=Part has been deleted')).toBeVisible();
});
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Register dialog handler before triggering dialog and use unique test data

The dialog handler should be registered before any action that might trigger a dialog, and each test should use unique data.

test('Ensure a part can be created and then deleted @hipanel-module-stock @manager', async ({ managerPage }) => {
    const partView = new PartCreateView(managerPage);
+    const uniquePartData = createPartData();
    await partView.navigate();
-    await partView.fillPartFields(partData);
+    await partView.fillPartFields(uniquePartData);
    await partView.save();
    await expect(managerPage.locator('text=Part has been created')).toBeVisible();

+    // Register dialog handler before clicking delete
+    managerPage.on('dialog', async dialog => await dialog.accept());
    await managerPage.click('text=Delete');
-    managerPage.on('dialog', async dialog => await dialog.accept());
    await expect(managerPage.locator('text=Part has been deleted')).toBeVisible();
});
📝 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
test('Ensure a part can be created and then deleted @hipanel-module-stock @manager', async ({ managerPage }) => {
const partView = new PartCreateView(managerPage);
await partView.navigate();
await partView.fillPartFields(partData);
await partView.save();
await expect(managerPage.locator('text=Part has been created')).toBeVisible();
await managerPage.click('text=Delete');
managerPage.on('dialog', async dialog => await dialog.accept());
await expect(managerPage.locator('text=Part has been deleted')).toBeVisible();
});
test('Ensure a part can be created and then deleted @hipanel-module-stock @manager', async ({ managerPage }) => {
const partView = new PartCreateView(managerPage);
const uniquePartData = createPartData();
await partView.navigate();
await partView.fillPartFields(uniquePartData);
await partView.save();
await expect(managerPage.locator('text=Part has been created')).toBeVisible();
// Register dialog handler before clicking delete
managerPage.on('dialog', async dialog => await dialog.accept());
await managerPage.click('text=Delete');
await expect(managerPage.locator('text=Part has been deleted')).toBeVisible();
});

});

40 changes: 40 additions & 0 deletions tests/playwright/page/PartCreateView.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import {expect, Locator, Page} from "@playwright/test";

export default class PartCreateView {
private page: Page;

public constructor(page: Page) {
this.page = page;
}

public async navigate() {
await this.page.goto('/stock/part/create');
}

public async fillPartFields(partData: any) {
await this.page.selectOption('select[id$=partno]', partData.partno);
await this.page.selectOption('select[id$=src_id]', partData.src_id);
await this.page.selectOption('select[id$=dst_ids]', partData.dst_id);
await this.page.fill('input[id$=serials]', partData.serials);
await this.page.fill('input[id$=move_descr]', partData.move_descr);
await this.page.fill('input[id$=price]', partData.price.toString());
await this.page.click(`li a[data-value=${partData.currency}]`);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add string interpolation safety for currency selector

Use proper string interpolation with quotes to prevent potential XSS or injection issues when concatenating values into selectors.

-await this.page.click(`li a[data-value=${partData.currency}]`);
+await this.page.click(`li a[data-value="${partData.currency}"]`);
📝 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
await this.page.click(`li a[data-value=${partData.currency}]`);
await this.page.click(`li a[data-value="${partData.currency}"]`);

await this.page.selectOption('select[id$=company_id]', partData.company_id);
}

public async save() {
await this.page.click('button:has-text("Save")');
}

public async addPart() {
await this.page.click("div.item:last-child button.add-item");
}

public async removePart() {
await this.page.click("div.item:last-child button.remove-item");
}

public async copyPart() {
await this.page.click("div.item:last-child button.copy");
}
}