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(commonTests, alert, combobox, dropdown, input-date-picker, modal, popover, tooltip): extracts a general test utility OpenClose for all components implementing openCloseComponent util #7379

Merged

Conversation

Elijbet
Copy link
Contributor

@Elijbet Elijbet commented Jul 25, 2023

Related Issue: #4968

Summary

Extracts a common test utility openClose for all components implementing openCloseComponent util.

Implemented on all relevant components.

  • alert
  • combobox
  • dropdown
  • input-date-picker
  • modal
  • popover
  • tooltip

@Elijbet Elijbet requested a review from a team as a code owner July 25, 2023 21:31
@github-actions github-actions bot added the testing Issues related to automated or manual testing. label Jul 25, 2023
@Elijbet Elijbet added the skip visual snapshots Pull requests that do not need visual regression testing. label Jul 25, 2023
@Elijbet Elijbet changed the title test(commonTests): extracts a general test utility emitsOpenCloseTransitionChainedEvents test(commonTests): extracts a general test utility emitsOpenCloseTransitionChainedEvents Jul 25, 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.

This is looking pretty sweet, @Elijbet! 🍬

Can you wire up this helper in other open/close component test suites?

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
Copy link
Contributor Author

Elijbet commented Jul 27, 2023

This is looking pretty sweet, @Elijbet! 🍬

Can you wire up this helper in other open/close component test suites?

Is it ok if I wire up this helper in other components separately from this PR?

I think it's going to bloat up the PR and make it hard to merge. I was hoping to address each case or several cases in a bunch within continuing PRs, so I can progressively adjust the util.

@jcfranco
Copy link
Member

Is it ok if I wire up this helper in other components separately from this PR?

I'd prefer to have the test helper cover all existing use cases from the get go. There's 6 other components that would need to be included, so I don't think it'll add much bloat since we're replacing any existing tests.

@Elijbet Elijbet marked this pull request as draft August 6, 2023 15:31
@Elijbet Elijbet added the refactor Issues tied to code that needs to be significantly reworked. label Sep 7, 2023
@Elijbet Elijbet added skip visual snapshots Pull requests that do not need visual regression testing. and removed skip visual snapshots Pull requests that do not need visual regression testing. labels Sep 12, 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.

🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️
🛠️🛠️🧪🧪🧪🛠️🛠️🧪🧪🛠️🛠️🛠️🧪🧪🛠️🛠️🧪🛠️🛠️🛠️🧪🛠️
🛠️🧪🛠️🛠️🛠️🛠️🧪🛠️🛠️🧪🛠️🧪🛠️🛠️🧪🛠️🧪🛠️🛠️🛠️🧪🛠️
🛠️🧪🛠️🛠️🛠️🛠️🧪🛠️🛠️🧪🛠️🧪🛠️🛠️🧪🛠️🧪🛠️🛠️🛠️🧪🛠️
🛠️🧪🛠️🛠️🛠️🛠️🧪🛠️🛠️🧪🛠️🧪🛠️🛠️🧪🛠️🧪🛠️🛠️🛠️🛠️🛠️
🛠️🛠️🧪🧪🧪🛠️🛠️🧪🧪🛠️🛠️🛠️🧪🧪🛠️🛠️🧪🧪🧪🛠️🧪🛠️
🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️🛠️

@Elijbet Elijbet merged commit 996e2b9 into main Sep 19, 2023
@Elijbet Elijbet deleted the elijbet/4968-add-common-test-util-for-all-OpenCloseComponents branch September 19, 2023 18:41
Elijbet added a commit that referenced this pull request Dec 21, 2023
…e commonTests helper (#8392)

Remove e2e tests that are covered by the dedicated `openClose
commonTests` helper.

**Related Issue:** #8391, #7379

## Summary

Remove e2e tests that are covered by dedicated `openClose commonTests`
helper to simplify test setup, ensure testing consistency across all
`openClose` components, and enhance test maintainability.
benelan pushed a commit that referenced this pull request Dec 22, 2023
…nClose` commonTests helper (#8472)

Remove e2e tests covered by the dedicated `openClose` commonTests
helper.

**Related Issue:** #8392, #7379

## Summary
Remove e2e tests covered by dedicated `openClose` commonTests helper to
simplify test setup, ensure testing consistency across all openClose
components, and enhance test maintainability.
jcfranco added a commit that referenced this pull request Sep 11, 2024
**Related Issue:** N/A

## Summary

Removes `beforeToggle` option as it is not being used by any test.

**Note:** It appears this option has not been used since it was
introduced by #7379.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Issues tied to code that needs to be significantly reworked. 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