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

Initial aria test util docs #7145

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Initial aria test util docs #7145

wants to merge 9 commits into from

Conversation

LFDanLu
Copy link
Member

@LFDanLu LFDanLu commented Oct 3, 2024

Closes

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

RSP

category: Concepts
---

# Testing
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually this testing page will have more than just documenting the aria test utils

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open to any suggestion as to the structure of this page and how in-depth the documentation should be. I figure it is easier and clearer to document each individual tester api in the RAC/RSP component page itself but happy to field opinions


See below for the full definition of the `User` object.

<ClassAPI links={testUtilDocs.links} class={testUtilDocs.exports.User} />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that PatternNames in the rendered type doesn't actually the true type, still trying to figure out how to handle this... It works just fine in the typescript
image

@rspbot
Copy link

rspbot commented Oct 4, 2024

@rspbot
Copy link

rspbot commented Oct 4, 2024

## API Changes

@react-aria/test-utils

/@react-aria/test-utils:User

 User {
   advanceTimer: UserOpts['advanceTimer']
   constructor: (UserOpts) => void
-  createTester: (T, ObjectOptionsTypes<T>) => ObjectType<T>
-  interactionType: UserOpts['interactionType']
-  user: any
+  createTester: (T, TesterOpts<T>) => Tester<T>
+  interactionType: UserOpts['interactionType'] = mouse
 }

/@react-aria/test-utils:UserOpts

 UserOpts {
   advanceTimer?: (number) => void | Promise<unknown>
-  interactionType?: 'mouse' | 'touch' | 'keyboard'
+  interactionType?: 'mouse' | 'touch' | 'keyboard' = mouse
 }

@react-spectrum/test-utils

/@react-spectrum/test-utils:User

 User {
   advanceTimer: UserOpts['advanceTimer']
   constructor: (UserOpts) => void
-  createTester: (T, ObjectOptionsTypes<T>) => ObjectType<T>
-  interactionType: UserOpts['interactionType']
-  user: any
+  createTester: (T, TesterOpts<T>) => Tester<T>
+  interactionType: UserOpts['interactionType'] = mouse
 }

/@react-spectrum/test-utils:UserOpts

 UserOpts {
   advanceTimer?: (number) => void | Promise<unknown>
-  interactionType?: 'mouse' | 'touch' | 'keyboard'
+  interactionType?: 'mouse' | 'touch' | 'keyboard' = mouse
 }

@LFDanLu LFDanLu marked this pull request as ready for review October 4, 2024 23:00
@@ -27,7 +27,6 @@
"peerDependencies": {
"@testing-library/react": "^15.0.7",
"@testing-library/user-event": "^13.0.0 || ^14.0.0",
"jest": "^29.5.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? won't this cause some warnings during yarn install?

/**
* Returns the combobox trigger button if present.
*/
get trigger(): HTMLElement {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if present would imply that this should be. So we should either update to that or change the sentence

get trigger(): HTMLElement | null

/**
* The index of the row to toggle selection for.
*/
index?: number,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if an index isn't supplied?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's either text OR index, maybe overloads would be more suitable?

return this._gridlist;
}

get rows() {
/**
* Returns the gridlist's rows if any.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these 'any' are fine because the array could be empty

let comboboxTester = testUtilUser.createTester('ComboBox', {root: getByTestId('test-combobox'), interactionType: 'keyboard'});

await comboboxTester.open();
expect(comboboxTester.listbox).toBeTruthy();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: a little more common check probably

Suggested change
expect(comboboxTester.listbox).toBeTruthy();
expect(comboboxTester.listbox).toBeInTheDocument();

let options = comboboxTester.options();
await comboboxTester.selectOption({option: options[0]});
expect(comboboxTester.combobox.value).toBe('One');
expect(comboboxTester.listbox).toBeFalsy();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(comboboxTester.listbox).toBeFalsy();
expect(comboboxTester.listbox).not.toBeInTheDocument();

let options = comboboxTester.options();
await comboboxTester.selectOption({option: options[0]});
expect(comboboxTester.combobox.value).toBe('One');
expect(comboboxTester.listbox).toBeFalsy();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(comboboxTester.listbox).toBeFalsy();
expect(comboboxTester.listbox).not.toBeInTheDocument();

or for users who have built their own components following the respective ARIA pattern specification. By using the ARIA specification for any given component pattern as a source of truth,
we can make assumptions about the existence of specific aria attributes that allow us to navigate the component's DOM structure. Similarly, we can also expect that the component
permits specific interaction patterns described by the ARIA pattern specification and thus accurately simulate those interactions, using the aforementioned aria attributes to target the proper node
within the component or to verify that the component's state has changed appropriately post-interaction. By providing utilities to simulate these standard interaction and getters that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
within the component or to verify that the component's state has changed appropriately post-interaction. By providing utilities to simulate these standard interaction and getters that
within the component or to verify that the component's state has changed appropriately post-interaction. By providing utilities to simulate these standard interactions and getters that

we can make assumptions about the existence of specific aria attributes that allow us to navigate the component's DOM structure. Similarly, we can also expect that the component
permits specific interaction patterns described by the ARIA pattern specification and thus accurately simulate those interactions, using the aforementioned aria attributes to target the proper node
within the component or to verify that the component's state has changed appropriately post-interaction. By providing utilities to simulate these standard interaction and getters that
allow the user to easily look up the subcomponents of the component itself, we hope to simplify the overall test writing experience, leading towards easier adoption.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
allow the user to easily look up the subcomponents of the component itself, we hope to simplify the overall test writing experience, leading towards easier adoption.
allow the user to easily look up the subcomponents of the component itself, we hope to simplify the overall test writing experience, leading to easier adoption.


These test utilities were inspired by various issues and observations that the maintainers of this library and consumers have experienced when writing tests against our components over the years. It is still very much
a work in progress so if you discover any issues or have any feedback please feel free to report them via [GitHub issues](https://github.com/adobe/react-spectrum/issues)! If you have implemented
any testing utilities yourself that you feel would be a good fit, we would be happy to field any pull request! Please read our [contributing guide](contribute.html)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
any testing utilities yourself that you feel would be a good fit, we would be happy to field any pull request! Please read our [contributing guide](contribute.html)
any testing utilities yourself that you feel would be a good fit, we would be happy to review any pull requests! Please read our [contributing guide](contribute.html)

### Setup

Once installed, you can access the `User` that `@react-aria/test-utils` provides in your test file as shown below. This user only needs to be initialized once and accepts two options: `interactionType` and `advanceTimer`. `interactionType` will
initialize what mode of interaction (mouse, keyboard, or touch) will be used by default. This can be overridden at the pattern tester or interaction execution level if required. `advanceTimer` accepts a function that when called should advance timers (real or fake)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does it mean to advance real timers?
is that just 'waiting' for x time?

@LFDanLu
Copy link
Member Author

LFDanLu commented Nov 14, 2024

TODO: JSDOCs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants