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

feat: Adds extra selectors to the test utils #2932

Merged

Conversation

orangevolon
Copy link
Contributor

@orangevolon orangevolon commented Oct 24, 2024

Description

Adds two new test utils selectors to each component wrapper.

  1. find[COMPONENT_NAME]ByTestId(testId: string)
  2. findAll[COMPONENT_NAME]s()

Depends on cloudscape-design/test-utils#74 (This is why the Github checks are failing)
Followed by #2944

Related links, issue #, if available: Test utils API improvements project

How has this been tested?

✅ This PR: #2944
💹 Copies including the JSDocs and the pluralizations have been reviewed by the content team.

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

codecov bot commented Oct 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.24%. Comparing base (041e7e5) to head (2bc4260).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2932   +/-   ##
=======================================
  Coverage   96.24%   96.24%           
=======================================
  Files         769      769           
  Lines       21722    21726    +4     
  Branches     7371     7430   +59     
=======================================
+ Hits        20907    20911    +4     
  Misses        762      762           
  Partials       53       53           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orangevolon orangevolon marked this pull request as ready for review October 30, 2024 08:11
@orangevolon orangevolon requested a review from a team as a code owner October 30, 2024 08:11
@orangevolon orangevolon requested review from michaeldowseza and removed request for a team October 30, 2024 08:11
@orangevolon orangevolon marked this pull request as draft October 30, 2024 15:24
@orangevolon orangevolon force-pushed the alamiami/adds-find-all-components branch from 2c6f204 to 369a0c5 Compare November 1, 2024 11:18
@orangevolon orangevolon marked this pull request as ready for review November 1, 2024 13:36
@orangevolon orangevolon changed the base branch from main to feat/test-utils-project-m2 November 4, 2024 10:39
@orangevolon orangevolon force-pushed the alamiami/adds-find-all-components branch from 95b2b79 to dbf99f5 Compare November 4, 2024 10:40
@orangevolon orangevolon force-pushed the alamiami/adds-find-all-components branch from dbf99f5 to be2ffae Compare November 5, 2024 12:22
@orangevolon orangevolon force-pushed the alamiami/adds-find-all-components branch 3 times, most recently from 44ecbba to a7dcd25 Compare November 6, 2024 13:46
common: {
// These components are not meant to be present in multiple instances in a single app.
// For this reason no findAll and findByTestId finders will be generated for them.
noExtraFinders: ['AppLayout', 'TopNavigation'],
Copy link
Contributor Author

@orangevolon orangevolon Nov 6, 2024

Choose a reason for hiding this comment

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

A bit more context on this: The "root" element of these two components behave differently. For example for the top navigation:

<div data-testid="assiged-by-user">
  <div>
    <div class="awsui-hashed-root-class">
      ...
    </div>
  </div>
</div>

So data-testid is the rootClass has are not assigned to the same element. This means findTopNavigationByTestId will never return any result and might make the users confused.

Since these two components are also not meant to be multiple in a single app, we decided to keep these two as exceptions. They won't receive findAllXX and findXXByTestId finders.

We'll address them separately later as it might need a change to the component itself.

@orangevolon orangevolon force-pushed the alamiami/adds-find-all-components branch from a7dcd25 to 50f534a Compare November 6, 2024 14:39
@orangevolon orangevolon force-pushed the alamiami/adds-find-all-components branch from 50f534a to 4406433 Compare November 13, 2024 13:59
package.json Outdated Show resolved Hide resolved
build-tools/tasks/test-utils.js Outdated Show resolved Hide resolved
src/__tests__/functional-tests/test-utils.test.tsx Outdated Show resolved Hide resolved
@orangevolon orangevolon removed the request for review from michaeldowseza November 14, 2024 10:09
@orangevolon orangevolon merged commit 579974a into feat/test-utils-project-m2 Nov 14, 2024
1 check passed
@orangevolon orangevolon deleted the alamiami/adds-find-all-components branch November 14, 2024 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants