feat: Adds findAll finder to test utils #330
Conversation
orangevolon
force-pushed
the
feat/adds-find-all-selectors-to-test-utils
branch
4 times, most recently
from
57fb6bc to
274c1dd
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #330 +/- ##
==========================================
- Coverage 93.97% 93.59% -0.38%
==========================================
Files 61 61
Lines 4312 4312
Branches 532 737 +205
==========================================
- Hits 4052 4036 -16
- Misses 260 276 +16 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
| @@ -1,16 +1,126 @@ | |||
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
There was a problem hiding this comment.
Can we add this script to https://github.com/cloudscape-design/build-tools?
There was a problem hiding this comment.
I assume you mean the whole file. For that the answer is yes, we can. There are minor differences between components and other three repos but I think it's totally doable.
There's also a lot more that we can centralize between the four repos, but IMO we need to discuss this in a separate thread.
|
|
||
| function getComponentSelectors(componentName: string) { | ||
| const componentNamePascalCase = pascalCase(componentName); | ||
| const findAllRegex = new RegExp(`findAll${componentNamePascalCase}.*`); |
There was a problem hiding this comment.
There is a lot of transformations happening for a test file. Wouldn't it be better straightforward to have some mapping instead, like:
const components = [
{
name: 'Board',
component: components.Board,
findName: 'findBoard',
findAllName: 'findAllBoard',
},
]
There was a problem hiding this comment.
Whatever practice we go for, we need to make it consistent all across the repos. So gotta have the map also in the components repo and three other repos as well.
I agree with the concern, but I think we need to fix this a bit differently and outside of the scope of this PR.
|
|
||
| function getComponentSelectors(componentName: string) { | ||
| const componentNamePascalCase = pascalCase(componentName); | ||
| const findAllRegex = new RegExp(`findAll${componentNamePascalCase}.*`); |
There was a problem hiding this comment.
Why is regex needed? Can a string comparison like target === findAll${componentNamePascalCase} work?
There was a problem hiding this comment.
Pluralization. findAlert --> findAlerts
orangevolon
force-pushed
the
feat/adds-find-all-selectors-to-test-utils
branch
from
274c1dd to
91a1d0f
Compare
|
|
||
| function getComponentSelectors(componentName: string) { | ||
| const componentNamePascalCase = pascalCase(componentName); | ||
| const findAllRegex = new RegExp(`findAll${componentNamePascalCase}.*`); |
Description
Adds two
findAll[COMPONENT_NAME]s()test utils selectors to each component wrapper.Similar to: cloudscape-design/components#3024
Depends on cloudscape-design/test-utils#74
Related links, issue #, if available: Test utils API improvements project
How has this been tested?
✅ Copies including the JSDocs and the pluralizations have been reviewed by the content team.
🧪 Unit tests added to both selectors and dom test utils.
Related links, issue #, if available: Test utils API improvements project
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.