test: [DBAAS1-1403] Cypress tests for Postgresql Synchronous Replication Advanced Configuration#13440
Conversation
…nced config, include nested/top level config
…tion advanced config
jdamore-linode
left a comment
jdamore-linode
left a comment
There was a problem hiding this comment.
Thanks Sakshi! I took a look at the failures and I think I have a rough understanding of what's going on. Feel free to follow up in Slack if you have any questions!
There was a problem hiding this comment.
nit: any chance we can remove this debug() call? I know it's not added by this PR but it looks like it slipped in here by mistake
| if ($form.find(`[name="${flatKey}"]`).length) { | ||
| cy.get(`[name="${flatKey}"]`).scrollIntoView(); | ||
| cy.get(`[name="${flatKey}"]`).should('be.visible').clear(); | ||
| cy.get(`[name="${flatKey}"]`).type(additionalConfigs[flatKey]); | ||
| } else { | ||
| // Fallback | ||
| ui.autocomplete.find().first().scrollIntoView(); | ||
| ui.autocomplete | ||
| .find() | ||
| .first() | ||
| .should('be.visible') | ||
| .clear() | ||
| .type(`${additionalConfigs[flatKey]}`); | ||
| } |
There was a problem hiding this comment.
I think this if/else block is responsible for the failures.
When I view the recording, I see that Cypress is typing '6000' into the "Add a Configuration Option" field rather than the "wal_sender_timeout" field. Then in the next iteration when it tries to add the "innodb_ft_min_token_size" config, it can't find the option in the autocomplete drop-down because '6000' was already present in the field:
I have a few thoughts:
- The root of the issue is that we're using jQuery-style functions like
$form.find(...)to query the state of the DOM. This is fragile because it only checks the state of the DOM at that moment in time and doesn't have any waiting/timeout logic like we have withcy.commands; in this case, the test is failing because thisifstatement is executing faster than Cloud Manager can re-render the drawer DOM in CI, causing the check to returnfalse - I don't understand what the fallback is accomplishing: I believe the intention is to type the configuration value into the right field, but as far as I can tell the first autocomplete field in the drawer will always be the "Add a Configuration Option" drop-down, so I don't think this fallback is doing what we want it to do anyway
I would suggest refactoring this in a way that uses Cypress's commands to assert the state of the drawer rather than using jQuery to attempt to check and respond to its state, e.g.:
cy.contains(flatKey)
.scrollIntoView()
.should('be.visible')
.parent()
.within(() => {
// This limits your selection to the div containing the configuration setting.
// Type into the text field, or click the toggle button, or perform whatever interaction is necessary here.
});There was a problem hiding this comment.
Thanks Joe, I have tried to address the above pointers.
Cloud Manager UI test results🔺 5 failing tests on test run #3 ↗︎
Details
TroubleshootingUse this command to re-run the failing tests: pnpm cy:run -s "cypress/e2e/core/linodes/smoke-linode-landing-table.spec.ts,cypress/e2e/core/parentChild/account-switching.spec.ts,cypress/e2e/core/cloudpulse/alerting-notification-channel-permission-tests.spec.ts" |
|||||||||||||||||||||||||||||
3 participants
Description 📝
Changes 🔄
List any change(s) relevant to the reviewer.
Target release date 🗓️
N/A
How to test 🧪
Run the test or some subset of test and confirm all actions can be performed.
Author Checklists
As an Author, to speed up the review process, I considered 🤔
👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support
As an Author, before moving this PR from Draft to Open, I confirmed ✅