Skip to content
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

Evaluate testing APIs that components should expose to client applications #265

Open
fredvisser opened this issue Jan 20, 2022 · 16 comments
Assignees
Labels
enhancement New feature or request

Comments

@fredvisser
Copy link
Contributor

fredvisser commented Jan 20, 2022

Copied from User Story 1537054: Evaluate testing APIs that components should expose to client applications

(See also the comments in that US)

It should be easy for a client application to write a test which includes a component in the page, configures it, interacts with it, and inspects its visual state.

We've had good success following Angular Material's test harness/page object pattern and we had many challenges with the testability of Smart elements. We should evaluate what testing APIs the nimble components need to expose to be delightful for client applications to consume them.

The primary goal for now is Angular client applications, but we should see if we can write this layer at the nimble-components level so that other frameworks could take advantage of it too.

Current status

A branch exists that includes:

  • a page object for the combobox
    • built using testing-library user events
    • includes unit tests that are passing
  • re-export the page object for Angular
    • includes unit tests that are not passing

A not building branch exists that shows what it looks like to use that page object in an Angular app.

Possible next steps

  1. fix angular tests
  2. write page objects for more components (see comments in this issue for some interesting ones)
  3. figure out cases where page objects might need to expose read APIs (e.g. breadcrumb)
  4. think about code sharing with playwright tests and how page objects would bubble up to apps.
    • see playwright testing library for ideas about locators
    • think about whether we could single source page objects for both testing frameworks despite them having different models for input events
@fredvisser fredvisser added the enhancement New feature or request label Jan 20, 2022
@rajsite rajsite self-assigned this Feb 18, 2022
@rajsite
Copy link
Member

rajsite commented Feb 24, 2022

Related: #362

@jattasNI
Copy link
Contributor

As part of this work it could be helpful to make the helpers in async-test-utilities available to clients via our public API. In this PR a client ended up duplicating one of those APIs.

@rajsite
Copy link
Member

rajsite commented Feb 28, 2022

As part of this work, we need to find a way to capture the async timing behavior: #362 (comment)
Wonder if it is possible to plugin to zonejs for some of that and if zonejs needs an issue for the slotchanged events

@jattasNI
Copy link
Contributor

jattasNI commented Mar 9, 2022

Another example use case appeared on this AzDO PR. The page object for the properties list component is explicitly firing an input event on the nimble-text-field in order to trigger its change event handler. Nimble might want to provide a higher level harness that sets a value and triggers the event.

Same for nimble-number-field (this was explicitly requested by the Argon team in a chat with me on 4/6/22). For the number field we should ensure it works well with both 'value' (string) and 'valueAsNumber' (numeric).

@rajsite
Copy link
Member

rajsite commented Mar 16, 2022

@gokulakrishnan-ni wanted to be able to simulate button click interactions in test. The approach that was used was not aware of disabled state. Pointed to the FAST approach which was to run the click method on the button in test which is disabled state aware: https://github.com/microsoft/fast/blob/231bf286ef7eb981e392d65651621ad4d325162a/packages/web-components/fast-foundation/src/button/button.spec.ts#L583

@rajsite
Copy link
Member

rajsite commented Mar 17, 2022

@jattasNI
Copy link
Contributor

jattasNI commented Apr 5, 2022

Some more good examples in the pageobject this PR. Specifically all the methods that operate on the nimble-select but have to do innerText.trim() to access the correct item in the drop down. Preferably the select would expose a page object that contained methods like selectOption() and getSelectOptions().

@atmgrifter00
Copy link
Contributor

In this PR, I had to write a pageObject method implementation to do some fairly specific event dispatching coupled with particular state changes, to achieve an analogous user interaction behavior. It might be beneficial to capture this sort of user interaction with the Combobox in a Nimble test harness.

@jattasNI
Copy link
Contributor

I'm closing #362 because that issue included a collection of tips that are too low-level to document but would be great candidates to include in a page object for nimble-tabs. If we work on this issue we should mine that discussion thread for ideas around fakeAsync and requestAnimationFrame.

@rajsite
Copy link
Member

rajsite commented Oct 10, 2022

Some interesting topics:

@rajsite
Copy link
Member

rajsite commented Oct 10, 2022

As part of this work it could be helpful to make the helpers in async-test-utilities available to clients via our public API. In this PR a client ended up duplicating one of those APIs.

A bit on the fence about exposing the waitMicrotask and waitTask helpers publically. Those aren't nimble-specific but general tools for the web. If sharing them is useful maybe they should be their own library.

Another way to look at the issue is that clients are needing to be aware of those implementation details of nimble controls (I make an update and need to waitMicrotask or waitTask for X ms). Talking to @atmgrifter00 hopefully user's needing to be aware of those details could be mitigated altogether with good page objects for the controls.

@jattasNI
Copy link
Contributor

@rajsite pointed out that the FAST element 2 refactor of the task queue now exposes a mode to process async tasks synchronously in tests. We could consider telling clients to use that mode (and maybe have our page objects only work in that mode) and avoid us manually managing the queue via processUpdates.

https://github.com/microsoft/fast/blob/master/packages/web-components/fast-element/src/observation/update-queue.ts#L116

@m-akinc
Copy link
Contributor

m-akinc commented Feb 24, 2023

Another use case from this PR:

  • get content slotted in default slot of the banner (aka get visible text for a banner with title-hidden set)

@jattasNI jattasNI mentioned this issue Mar 7, 2023
1 task
@jattasNI jattasNI mentioned this issue May 1, 2023
1 task
@jattasNI
Copy link
Contributor

Playwright page objects might be a good way to address #1339. The way to check if an element is disabled changes depending on the type of element and that logic could be encapsulated in a page object.

@jattasNI
Copy link
Contributor

I spent an innovation day trying to write a page object for select. Got it fairly far along but didn't end up submitting it, so linking it here as a starting point if we ever prioritize this. #1500

@jattasNI
Copy link
Contributor

jattasNI commented Jul 31, 2024

We have slowly made progress on this issue by adding a few new page objects. But there are open questions on best practices that we should reach consensus on and document somewhere. Some of this was discussed in this thread.

  1. I believe we are starting to have "internal" page object APIs (those useful for writing Nimble component tests) vs public page object APIs (those only useful for clients writing tests that use Nimble components) but don't have consistent ways to distinguish them.
    • Proposal: public and internal APIs in same file, just add @internal comments
  2. should we have different documentation policies for those two types of APIs?
    • Proposal: yes, public methods should have documentation but internal methods don't need it
  3. should we have different testing policies for those two types of APIs?
    • Proposal: public methods should have dedicated page object tests so we can ensure they remain tested even if no nimble component tests need them
  4. if yes, what should be the conventions for where the tests live
    • Proposal: separate file for public page object tests: table/testing/tests/table.pageobject.spec.ts (while component tests are in table/tests/table.spec.ts: note the extra testing folder). This allows /tests to be excluded from package contents and also for clients to import page objects from a different /testing entry point than the component itself and potentially enforce via linting that production code doesn't rely on page objects
  5. Nimble Angular page objects can extend Nimble components page objects when needed
  6. should page objects make assumptions about the async environment that they're running in? (particularly for Angular tests that can be fakeAsync and should call processUpdates not waitForUpdatesAsync)
    • could try moving to processUpdates more generally when possible since it should work in both contexts and will be faster (avoids 16ms wait)
    • we should generally discourage fakeAsync tests and find patterns to avoid them for common things (e.g. debouncing input)
    • when non-fakeAsync-compatible page objects methods need to exist we shouldn't hesitate to add them. If a fakeAsync test then needs to use that capability, it could start by forking the Nimble implementation into its own SLE page object and modifying it to include the relevant fakeAsync awaits / ticks / processUpdates / etc. If that's generally useful it could move into the nimble-angular page object with a fakeAsync-focused name

In a discussion today the team agreed on items 1-5 and want to continue refining the items in 6. I will capture those guidelines in CONTRIBUTING and update some page objects to follow them in an upcoming PR.

@jattasNI jattasNI added the triage New issue that needs to be reviewed label Jul 31, 2024
@m-akinc m-akinc removed the triage New issue that needs to be reviewed label Aug 6, 2024
msmithNI added a commit that referenced this issue Aug 12, 2024
…2343)

# Pull Request

## 🤨 Rationale

Followup for #2183 .
Once I started uptaking the built-in label change in SLE, I saw several
page objects that will need a Nimble-provided way to get the label text
(so the SLE page objects don't need to query inside the Nimble shadow
roots).

## 👩‍💻 Implementation

- Add `getLabelText()` to select and combobox page objects

## 🧪 Testing

- Add new test files for these 2 page objects, with 2 tests each for
`getLabelText()`.
- These follow the new guidance in [the discussion in
#265](#265 (comment))

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Backlog
Development

No branches or pull requests

5 participants