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

test: set up disabled helper to run a test per use case #7089

Merged
merged 17 commits into from
Jun 22, 2023

Conversation

Elijbet
Copy link
Contributor

@Elijbet Elijbet commented Jun 2, 2023

Related Issue: #N/A

Summary

Refactor disabled test helper to help mitigate timeouts.

Splitting up our common test helpers into individual tests helps mitigate timeouts that occurred when a helper's coverage ended up taking longer than the default test timeout.

The updated doc reflects how to use the helpers in E2E tests.

@Elijbet Elijbet added testing Issues related to automated or manual testing. skip visual snapshots Pull requests that do not need visual regression testing. labels Jun 2, 2023
@github-actions
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Jun 13, 2023
@Elijbet Elijbet removed the Stale Issues or pull requests that have not had recent activity. label Jun 14, 2023
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Looking good! 😎

packages/calcite-components/src/tests/commonTests.ts Outdated Show resolved Hide resolved
packages/calcite-components/src/tests/commonTests.ts Outdated Show resolved Hide resolved
packages/calcite-components/src/tests/commonTests.ts Outdated Show resolved Hide resolved
packages/calcite-components/src/tests/commonTests.ts Outdated Show resolved Hide resolved
packages/calcite-components/src/tests/commonTests.ts Outdated Show resolved Hide resolved
packages/calcite-components/src/tests/commonTests.ts Outdated Show resolved Hide resolved
packages/calcite-components/src/tests/commonTests.ts Outdated Show resolved Hide resolved
packages/calcite-components/src/tests/commonTests.ts Outdated Show resolved Hide resolved
packages/calcite-components/src/tests/commonTests.ts Outdated Show resolved Hide resolved
@Elijbet Elijbet marked this pull request as ready for review June 21, 2023 20:04
@Elijbet Elijbet requested a review from a team as a code owner June 21, 2023 20:04
@geospatialem geospatialem requested a review from jcfranco June 21, 2023 21:38
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for helping out with this one. 🎉

packages/calcite-components/src/tests/commonTests.ts Outdated Show resolved Hide resolved
packages/calcite-components/src/tests/commonTests.ts Outdated Show resolved Hide resolved
const [shadowFocusableCenterX, shadowFocusableCenterY] = await page.$eval(tabFocusTarget, (element: HTMLElement) => {
const focusTarget = element.shadowRoot.activeElement || element;
const rect = focusTarget.getBoundingClientRect();
if (options.focusTarget === "none") {
Copy link
Member

Choose a reason for hiding this comment

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

This test still uses resources to set up the test when the focus target is not none. I think this one needs to get merged into it("prevents focusing via keyboard and mouse") where you have different focus assertions depending on options.focusTarget.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Take a look at the merge of those 2 tests now, does the combined test need a new title?

Copy link
Member

Choose a reason for hiding this comment

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

Looks great, thanks!

@Elijbet Elijbet merged commit abc0b3b into master Jun 22, 2023
@Elijbet Elijbet deleted the elijbet/set-up-disabled-helper-to-stabiize-test-runs branch June 22, 2023 16:25
@github-actions github-actions bot added this to the 2023 June patch priorities milestone Jun 22, 2023
benelan added a commit that referenced this pull request Jun 27, 2023
…issue-template-checkbox

* origin/master: (32 commits)
  ci: double build cc when publishing to workaround stencil types bug (#7227)
  build: bump package versions back to 1.5.0-next.5 (#7228)
  chore: release latest (#7144)
  ci: hardcode release version due to reverting feat (#7225)
  fix(tree-item): ensure expanded tree-item is displayed when expanded and made visible (#7216)
  ci(release-please): pin action version and allow manually running action (#7222)
  fix(input, input-number): allows numeric characters. (#7213)
  build(t9n): generate json file containing t9n values (#7214)
  chore: release next
  fix(combobox, dropdown, input-date-picker, input-time-picker, popover, tooltip): Prevent repositioning from affecting other floating components (#7178)
  build: update browserslist db (#7192)
  build: ignore node_modules and build outputs when watching for changes during stencil tests (#7209)
  test: set up `disabled` helper to run a test per use case (#7089)
  ci: set design complete label conditionals (#7206)
  chore: release next
  fix(list): update selectedItems property on all item selection changes (#7204)
  chore: fix sorting logic for t9nmanifest entries (#7203)
  fix(radio-button):  focuses first focusable radio-button element in group. (#7152)
  chore: release next
  fix(alert): update alert queue when an alert is removed from the DOM (#7189)
  ...
jcfranco added a commit that referenced this pull request Jun 27, 2023
…e" focus targets (#7235)

**Related Issue:** N/A

## Summary

Restores [assertions for tab/mouse focusing on disabled
components](https://github.com/Esri/calcite-components/blame/6777b06c5dbad5441ed651d676cd9562d79e6986/packages/calcite-components/src/tests/commonTests.ts#L1044-L1064)
removed in the [`disabled` test helper
refactor](#7089).
benelan pushed a commit that referenced this pull request Jun 28, 2023
…e" focus targets (#7235)

**Related Issue:** N/A

## Summary

Restores [assertions for tab/mouse focusing on disabled
components](https://github.com/Esri/calcite-components/blame/6777b06c5dbad5441ed651d676cd9562d79e6986/packages/calcite-components/src/tests/commonTests.ts#L1044-L1064)
removed in the [`disabled` test helper
refactor](#7089).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip visual snapshots Pull requests that do not need visual regression testing. testing Issues related to automated or manual testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants