-
Notifications
You must be signed in to change notification settings - Fork 17
EVEREST-1719 - Updated table.ts and added restore-new-cluster.e2e.ts #1229
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
base: main
Are you sure you want to change the base?
Conversation
@fabio-silva @dianabirs @tplavcic Can you please review this PR. Thanks |
|
||
// Click "Continue" 4 times | ||
for (let i = 0; i < 4; i++) { | ||
await expect( |
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.
maybe await moveForward(page);
?
deleteDbCluster, | ||
gotoDbClusterBackups, | ||
gotoDbClusterRestores, | ||
restartDbCluster, | ||
resumeDbCluster, | ||
suspendDbCluster, |
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.
These are unused
deleteDbCluster, | |
gotoDbClusterBackups, | |
gotoDbClusterRestores, | |
restartDbCluster, | |
resumeDbCluster, | |
suspendDbCluster, | |
deleteDbCluster, | |
gotoDbClusterBackups, | |
gotoDbClusterRestores, |
.locator('.MuiTableRow-root') | ||
.filter({ hasText: name }) | ||
.getByTestId('MoreHorizIcon') | ||
.click({ timeout: 10000 }); | ||
.first(); |
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.
Which text is appearing twice? I don't like this because .first()
is not deterministic and you don't know what is going to end up first so it is better that the test fails if there are some unexpected strings.
If we can control cluster names make them unique.
If substring is a problem consider using regex like .filter({ hasText: name RegExp(
^${name}$) })
.
test.skip(!shouldExecuteDBCombination(db, size)); | ||
test.describe.configure({ timeout: 720000 }); | ||
|
||
const clusterName = `${db}-${size}-schbkp`; |
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.
Please correct the cluster name to something unique to this test, maybe ${db}-${size}-rest-new
.
await prepareTestDB(clusterName, namespace); | ||
}); | ||
|
||
test(`Create backup schedules [${db} size ${size}]`, async ({ page }) => { |
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.
Please use on-demand backups instead of scheduled backups, because that way we don't need to wait for backups to get scheduled and fired in tests (much faster) and I don't see how restore from scheduled backup is different than restore from on demand full backup.
Thanks for the feedback! I'm currently working on another high-priority task and plan to get back to this PR later next week. |
Updated table.ts to fix row selection issues and created tests for restore to a new cluster. Some code is commented out as it requires additional fixes before merging.
Jira Ticket: Everest-1719