-
Notifications
You must be signed in to change notification settings - Fork 17
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
test: Fix component delete function in UE #165
base: main
Are you sure you want to change the base?
Conversation
// eslint-disable-next-line no-await-in-loop | ||
await adaptiveFormPath.first().click(); | ||
|
||
// eslint-disable-next-line max-len,no-await-in-loop | ||
const isComponentSelected = await frame.locator(this.componentSelectorValidation(component)).isVisible(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
componentSelectorValidation is coupled with text-input. this makes the code confusing to understand.
we should just have an API to get the selected component from the content tree and then decide whether it's a form or not.
const isAdaptiveFormSelected = await frame.locator(this.selectors.selectedAdaptiveFormWithComponent).isVisible(); | ||
|
||
// eslint-disable-next-line no-await-in-loop | ||
if (!isComponentSelected && isAdaptiveFormSelected) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
avoid too many eslint disables. better to take out these lines as a separate func outside while
|
||
// eslint-disable-next-line no-await-in-loop | ||
if (!isComponentSelected && isAdaptiveFormSelected) { | ||
const expandButton = frame.locator(this.getExpandCollapseButton('Expand')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets have a common method to expand the component in the tree
// eslint-disable-next-line no-await-in-loop | ||
await frame.locator(`li[data-resource*="/${component}"]`).click(); | ||
// eslint-disable-next-line no-await-in-loop,max-len | ||
expect(await frame.locator(this.componentSelectorValidation(component)).isVisible()).toBe(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets have a isComponentSelected
method.
if (await frame.locator(this.selectors.adaptiveFormPathInContentTree).isVisible()) { | ||
break; | ||
} | ||
|
||
// eslint-disable-next-line no-await-in-loop | ||
await frame.locator(this.selectors.deleteButton).click(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
common method to deleteComponent is required here.
The tests code is quite unreadable at present and thus, not maintainable.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requires refactoring.
Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after):
Fix #
Test URLs: