ni / nimble

The NI Nimble Design System
https://nimble.ni.dev
MIT License
30 stars 8 forks source link

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

Open fredvisser opened 2 years ago

fredvisser commented 2 years ago

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 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
rajsite commented 2 years ago

Related: https://github.com/ni/nimble/issues/362

jattasNI commented 2 years ago

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 commented 2 years ago

As part of this work, we need to find a way to capture the async timing behavior: https://github.com/ni/nimble/issues/362#issuecomment-1054457992 Wonder if it is possible to plugin to zonejs for some of that and if zonejs needs an issue for the slotchanged events

jattasNI commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

We should be aware of how users are expected to use JS-based test harnesses in test frameworks like test cafe: https://ni.visualstudio.com/DevCentral/_git/srd-semi-ses-software/pullrequest/278853?discussionId=2905287&_a=files&path=%2fArgon%2fServices%2fEnd2EndTests%2ftests%2fpage-objects%2flogin-page.js

jattasNI commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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 commented 1 year ago

Some interesting topics:

rajsite commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

Another use case from this PR:

jattasNI commented 1 year ago

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 commented 9 months ago

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 commented 1 month ago

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.