test: [M3-8914] - POC: Centralized Locators for Linode Page in Cypress UI Automation Framework

[subsingh-akamai]

Description 📝

The objective of this proof of concept (PoC) task is to enhance the UI automation framework in Cypress by implementing centralized locators for the Linode page. This will involve:

• Identifying all the locators used on the Linode page used in test cypress/e2e/core/linodes/smoke-linode-landing-table.spec.ts
• Creating a centralized repository or file to store these locators.
• Refactoring existing Cypress test scripts to utilize the centralized locators.

Changes 🔄

As part of POC, I have updated of locators in test cypress/e2e/core/linodes/smoke-linode-landing-table.spec.ts
Below 2 files are created under cypress/support/ui/locators to store locators used in above test

• common-locators.ts
• linode-locators.ts

How to test 🧪

yarn cy:run -s cypress/e2e/core/linodes/smoke-linode-landing-table.spec.ts

Verification steps

When test executed using this command yarn cy:run -s cypress/e2e/core/linodes/smoke-linode-landing-table.spec.tsall tests should pass successfully.

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
➕ Adding a changeset
🔐 Removing all sensitive information from the code and PR description

• I have read and considered all applicable items listed above.

[github-actions]

Coverage Report: ✅
Base Coverage: 86.96%
Current Coverage: 86.96%

[linode-gh-bot]

Cloud Manager UI test results

🎉 469 passing tests on test run #2 ↗︎

❌ Failing	✅ Passing	↪️ Skipped	🕐 Duration
0 Failing	469 Passing	2 Skipped	102m 42s

[bnussman-akamai]

Looks much cleaner in the test file! 🧹

Generally speaking, I think I prefer the style of using functions likefindByLabelText, findByDisplayValue, ... rather than selectors. I like functions like findByLabelText because they help encourage our application code to use correct semantics. I worry that using selectors could promote unnecessary test-ids too.

I like the clean up here, but I also don't mind keeping tests verbose. For example, I like

cy.findByLabelText('Toggle display').should('be.visible').should('be.enabled').click()

over

getVisible(listOfLinodesTableHeader.toggleDisplayButton).should('be.enabled').click();

because it is very clean what it is selecting on and doing.

[jdamore-linode]

One quick technical note: we can select any element by its aria-label value using cy.findByLabel, so I don't think we need any locators that select specifically using the aria-label attribute. For linodeActionMenu and linodeActionMenuButton, however, we have ui.actionMenu.findByTitle() which is the preferred way to select the action menus in the tests.

[jdamore-linode]

Thanks for the hard work on this @subsingh-akamai! I'm optimistic about this and think these can serve as a valuable foundational piece in our test suite.

One thing to emphasize is that using cy.get() in the tests should really only be done as a last resort. Generally if there's something we need to repeatedly use cy.get() for, we might want to consider adding data-qa-* or data-testid test attributes in the component, and adding a UI helper in support/ui instead. But between our UI helpers, Cypress testing library functions, and chains like .within() and .find() that make it possible to narrow down selections, we don't have to resort to cy.get very often. (smoke-linode-landing-table uses cy.get a lot because it's one of our oldest specs. It's a great spec to use for this POC because of that, but not the best in terms of demonstrating how a test should ideally be written)

• smoke-linode-landing-table looks a lot cleaner with the locators!
• Do you have any thoughts about documenting any of the locators? Maybe documenting each one could be excessive, but at least adding some doc comments to the objects might be helpful.
• I'm not sure about using these for labels or other UI strings; I think they should be limited to DOM selectors. For example, I don't think ui.button.findByTitle(emptyLinodePage.createLinodeButton) is any clearer to write, read, or maintain than ui.button.findByTitle('Create Linode'). Things like error messages and tooltip strings would probably be better defined in support/constants when useful.
• Any ideas about improving discoverability of these? I fear test development velocity could inadvertently get hurt if we have to stop and do a code search across the support/ui/locators folder every time we use cy.get to check if a locator exists for what we need. Do you think there are conventions or patterns we could use that would make it easier to predict where and how a locator might be defined?
• Do you envision any guidelines or requirements for defining new locators? Should we expect developers to define new locators every time they use cy.get() and a locator doesn't already exist?
  • More generally, how widely to do you envision these being used? Throughout the entire test suite? Only in specific circumstances or places?
• My biggest worry overall is from a test debugging standpoint. When we get a test failure in CI, we see a message like Expected to find element: [data-testid="menu-item-Linodes"][href="/linodes"], but never found it. The first thing I do to pinpoint the source of the issue is Ctrl + F in the spec and find that string, but now there's a layer of indirection where we'll have to search for that selector in support/ui/locators any time a test failure involves one of these locators. This is also an issue for our page objects in support/ui/pages, but we can factor that risk into our decision to use objects on a case-by-case basis which isn't true for the locators if we plan to use them throughout the entire suite.

Overall this is really nice work and has potential to really improve the way our tests are developed and organized! I think there's still a lot of room for discussion to better define when and how these should be applied but the code as-is looks great. Thanks again for taking this on Subodh!