You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/09/22 18:04:29 UTC
[GitHub] [superset] michael-s-molina opened a new issue #16792: [Proposal] Superset Frontend Testing Guidelines
michael-s-molina opened a new issue #16792:
URL: https://github.com/apache/superset/issues/16792
_I just opened this discussion to kick off the frontend testing guide. I'm just pasting some points to start thinking about the structure of the document._
- Gather RTL and Cypress best practices material
- Collect real examples from our codebase
- Avoid nesting tests https://kentcdodds.com/blog/avoid-nesting-when-youre-testing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-930146850
### Cypress
I am not much of a Cypress expert and I am taking what I think are the most relevant points for us from the official documentation, as well as adding some thoughts from my times working with it.
#### Do not use Cypress :)
Do not use Cypress when RTL can do it better and faster. Many of the Cypress tests that we have right now can be ported into RLT that is much faster and gives a more immediate feedback. Cypress should be used mainly for processes from A to B, B to C, etc. replicating the user experience, positive and negative flows.
#### Isolated and standalone tests
Tests should never rely on other tests to pass. This might be hard when a single user is used for testing as data will be stored into the database. At the end of each test we should reset the database. This discussion goes a bit out of the scope of the testing guidelines but might be worth pursuing.
#### Cleaning state
Cleaning the state of the application, such as resetting the db, or in general any state that might affect consequent tests, should always be done in the `beforeEach` hook and never in the `afterEach` one as the `beforeEach` is guaranteed to run, while the test might never reach the point to run the `afterEach`hook.
#### Unnecessary use of `cy.wait`
- Unnecessary when using `cy.request()` as it will resolve when a response is received from the server
- Unnecessary when using `cy.visit()` as it resolves only when the page fires the `load` event
- Unnecessary when using `cy.get()`. When the selector should wait for a request to happen, aliases would come handy. For example:
```
cy.intercept('GET', '/users', [{ name: 'Maggy' }, { name: 'Joan' }]).as(
'getUsers'
)
cy.get('#fetch').click()
cy.wait('@getUsers') // <--- wait explicitly for this route to finish
cy.get('table tr').should('have.length', 2)
```
#### Resilience
Prefer `data-*` attributes to isolate selectors from eventual CSS, JS or text content changes. Creating dedicated data selectors specifically for Cypress might increase the boilerplate a bit but comes with the advantage of being super resilient while making it apparent that the code is being tested. For example:
`cy.get('[data-cy=submit]').click()`
Alternatively using text might work but it is not as resilient. For example:
`cy.contains('Submit').click()`
#### Materials:
https://docs.cypress.io/guides/references/best-practices
https://www.youtube.com/watch?v=5XQOK0v_YRE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-949605378
> Hi! Sorry for chiming in while this is being voted on, but I think it's important to add that we should make sure not to change the codebase in order to fix a test. The only changes that should be added to code regarding tests should be adding a test-id if absolutely necessary.
When we use TDD we are always modifying the codebase in order to fix a test. I'm assuming you're talking about specific types of changes that should be avoided. To make this distinction clearer, can you give an example of a change in our codebase that is currently being made to fix a test?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-938673521
> I'm not sold on the "Avoid nesting when you're testing" point. I personally like the abstractions and groupings and dryer code that comes from that long-used pattern. If most other people are really in favor of this new pattern, then that's cool, but it's not my first choice.
No problem! I personally prefer to avoid `describe` because of the points described in Kent C. Dodds article but I'm also fine if the majority chooses to use it.
> Can we add more explanation to these points so that people reading in the future also understand the why:
Yes! Will do.
> Can we instead of discouraging people from using Cypress just relay the difference between end-to-end and integration tests? We still need people to continue to write end to end tests. :)
I was reading again and you're right! We can make it clear that we're favoring RTL for integration tests when possible but Cypress remains essential for e2e tests. Nice catch!
> when does the afterEach not run? I think it's better to clean up after yourself than after someone else. :)
@geido Can you detail this?
> Another common antipattern is that people will add classes or ids just for testing. I know we're talking about using roles and test-ids, etc, but do you think we can explicitly discourage adding anything into code for testing except for a test-id?
I think so. Do you have a sentence in mind to make it more clear? You can reply here or edit the issue's description directly 😉
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-937318028
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-932337972
One small nit... the philosophy of using data attributes for Cypress is accurate, but thus far we've been preferring `data-test` instead of `data-cy` as outlined above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-946968561
I know this is currently being voted on, but were there other opinions on the no `describe` nesting recommendation? I still personally like to write my tests like a tree and keep splitting them in to two cases. It's much easier to read that with nesting rather than flat. https://github.com/preset-io/superset/blob/43b92b220f465a08c6322b0ee58aaa0768e8e69[…]/spec/javascripts/dashboard/components/PropertiesModal_spec.jsx
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho closed issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho closed issue #16792:
URL: https://github.com/apache/superset/issues/16792
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-937319674
Can we add more explanation to these points so that people reading in the future also understand the why:
>Usage of waitFor
Do not use it for elements that can be queried with find
It should only have one assertion at a time
<hr>
Can we instead of discouraging people from using Cypress just relay the difference between end-to-end and integration tests? We still need people to continue to write end to end tests. :)
<hr>
>Cleaning state
Cleaning the state of the application, such as resetting the DB, or in general, any state that might affect consequent tests should always be done in the beforeEach hook and never in the afterEach one as the beforeEach is guaranteed to run, while the test might never reach the point to run the afterEach hook.
when does the afterEach not run? I think it's better to clean up after yourself than after someone else. :)
<hr>
Another common antipattern is that people will add classes or ids just for testing. I know we're talking about using roles and test-ids, etc, but do you think we can explicitly discourage adding anything into code for testing except for a test-id?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-932358347
> e've been preferring `data-tes`
Thanks for the review. Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-930059087
### RTL
...continuing on the topic of RTL
#### Simplicity
I think it's often overlooked how much simplicity is important in test cases. I believe clear, dumb code should always be preferred over complex one. The test cases should be pretty much standalone, and should not involve any external logic if not absolutely necessary. That's because you want the corpus of the tests to be easy to read and understandable at first sight.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-930059087
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-937319674
Can we add more explanation to these points so that people reading in the future also understand the why:
>Usage of waitFor
Do not use it for elements that can be queried with find
It should only have one assertion at a time
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-940522408
Thanks @geido that article makes sense in the context of Cypress and interacting with the test after it has ran. I think it would be helpful to link that article in the docs, too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-928009954
### RTL
> https://kentcdodds.com/blog/avoid-nesting-when-youre-testing
+1
> Our test files use the test suffix. We should migrate those that use the spec suffix.
+1
#### Priorities
I think priorities should be remarked as it is a common mistake:
https://testing-library.com/docs/queries/about/#priority
#### Using `act` unnecessarily
> render and fireEvent are already wrapped in act
https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning
#### Using `*ByRole`
By using the `name` option we can point to the items by their accessible name.
`screen.getByRole('button', {name: /hello world/i})`
Roles reference: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles
#### Prefer userEvent over fireEvent
The user-event library better represents the actual user actions.
@testing-library/user-event
#### Usage of `waitFor`
Do not use it for elements that can be queried with `find`
It should only have one assertion at a time
Do not perform side-effects actions inside `waitFor`. Use it only for assertions
#### Materials:
https://kentcdodds.com/blog/common-mistakes-with-react-testing-library
.....to be continued
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-928009954
### RTL
> https://kentcdodds.com/blog/avoid-nesting-when-youre-testing
+1
> Our test files use the test suffix. We should migrate those that use the spec suffix
+1
#### Priorities
I think priorities should be remarked as it is a common mistake:
https://testing-library.com/docs/queries/about/#priority
#### Using `act` unnecessarily
> render and fireEvent are already wrapped in act
https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning
#### Using `*ByRole`
By using the `name` option we can point to the items by their accessible name.
`screen.getByRole('button', {name: /hello world/i})`
Roles reference: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles
#### Prefer userEvent over fireEvent
The user-event library better represents the actual user actions.
@testing-library/user-event
#### Usage of `waitFor`
Do not use it for elements that can be queried with `find`
It should only have one assertion at a time
Do not perform side-effects actions inside `waitFor`. Use it only for assertions
#### Materials:
https://kentcdodds.com/blog/common-mistakes-with-react-testing-library
.....to be continued
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-930059087
### RTL
...continuing on the topic of RTL
#### Simplicity
I believe it's often overlooked how much simplicity is important in test cases. I believe clear, dumb code should always be preferred over complex ones. The test cases should be pretty much standalone, easy to read, should not involve any external logic if not absolutely necessary. That's because you want the corpus of the tests to be understandable at first sight.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
etr2460 commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-932373918
One other comment, if you could link an example test file or two to the guidelines (ideally a direct link to the current sha so it never changes) that could be helpful. Sometimes showing works better than telling (and honestly, i love being able to copy a test file as a template for the new one i'm making)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-949600123
> I know this is currently being voted on, but were there other opinions on the no `describe` nesting recommendation?
It seems people are fine with this. At least nobody else raised concerns so far and it seems this pattern is already being used by newer tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho closed issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho closed issue #16792:
URL: https://github.com/apache/superset/issues/16792
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-930146850
### Cypress
I am not much of a Cypress expert and I am taking what I think are the most relevant points for us from the official documentation, as well as adding some thoughts from my times working with it.
#### Do not use Cypress :)
Do not use Cypress when RTL can do it better and faster. Many of the Cypress tests that we have right now can be ported into RLT that is much faster and gives a more immediate feedback. Cypress should be used mainly for processes from A to B, B to C, etc. replicating the exact user experience, positive and negative flows.
#### Isolated and standalone tests
Tests should never rely on other tests to pass. This might be hard when a single user is used for testing as data will be stored into the database. At the end of each test we should reset the database. This discussion goes a bit out of the scope of the testing guidelines but might be worth pursuing.
#### Cleaning state
Cleaning the state of the application, such as resetting the db, or in general any state that might affect consequent tests, should always be done in the `beforeEach` hook and never in the `afterEach` one as the `beforeEach` is guaranteed to run, while the test might never reach the point to run the `afterEach`hook.
#### Unnecessary use of `cy.wait`
- Unnecessary when using `cy.request()` as it will resolve when a response is received from the server
- Unnecessary when using `cy.visit()` as it resolves only when the page fires the `load` event
- Unnecessary when using `cy.get()`. When the selector should wait for a request to happen, aliases would come handy. For example:
```
cy.intercept('GET', '/users', [{ name: 'Maggy' }, { name: 'Joan' }]).as(
'getUsers'
)
cy.get('#fetch').click()
cy.wait('@getUsers') // <--- wait explicitly for this route to finish
cy.get('table tr').should('have.length', 2)
```
#### Resilience
Prefer `data-*` attributes to isolate selectors from eventual CSS, JS or text content changes. Creating dedicated data selectors specifically for Cypress might increase the boilerplate a bit but comes with the advantage of being super resilient while making it apparent that the code is being tested. For example:
`cy.get('[data-cy=submit]').click()`
Alternatively using text might work but it is not as resilient. For example:
`cy.contains('Submit').click()`
#### Materials:
https://docs.cypress.io/guides/references/best-practices
https://www.youtube.com/watch?v=5XQOK0v_YRE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-930059087
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-937319674
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-939866880
> > when does the afterEach not run? I think it's better to clean up after yourself than after someone else. :)
>
> @geido Can you detail this?
@michael-s-molina @eschutho The official documentation goes in great length explaining the logic behind this. It is not only the fact that the `afterEach` might not be reached (for instance, by refreshing Cypress in the middle of the test you would build a partial state) but also to keep the ability to work with dangling state https://docs.cypress.io/guides/references/best-practices#Using-after-or-afterEach-hooks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-952178219
I believe @lyndsiWilliams was referring to adding selectors such as ids and classes that are only used for tests. That test-ids are preferable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-944260350
> Can we add more explanation to these points so that people reading in the future also understand the why:
Done
> Can we instead of discouraging people from using Cypress just relay the difference between end-to-end and integration tests? We still need people to continue to write end to end tests. :)
Done
Also added a `References` section.
I'll send an email to the dev list about this proposal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-946978485
Hi! Sorry for chiming in while this is being voted on, but I think it's important to add that we should make sure not to change the codebase in order to fix a test. The only changes that should be added to code regarding tests should be adding a test-id if absolutely necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-937319674
Can we add more explanation to these points so that people reading in the future also understand the why:
>Usage of waitFor
Do not use it for elements that can be queried with find
It should only have one assertion at a time
<hr>
Can we instead of discouraging people from using Cypress just relay the difference between end-to-end and integration tests? We still need people to continue to write end to end tests. :)
<hr>
>Cleaning state
Cleaning the state of the application, such as resetting the DB, or in general, any state that might affect consequent tests should always be done in the beforeEach hook and never in the afterEach one as the beforeEach is guaranteed to run, while the test might never reach the point to run the afterEach hook.
when does the afterEach not run? I think it's better to clean up after yourself than after someone else. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
etr2460 commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-932370872
Top level comment, do we want to summarize a list of Dos and Don'ts like Evan did for the emotion guidelines at the top? Not sure if that pattern always works
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
michael-s-molina edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-932358347
> One small nit... the philosophy of using data attributes for Cypress is accurate, but thus far we've been preferring data-test instead of data-cy as outlined above.
Thanks for the review. Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-930059087
### RTL
...continuing on the topic of RTL
#### Simplicity
I think it's often overlooked how much simplicity is important in test cases. I believe clear, dumb code should always be preferred over complex one. The test cases should be pretty much standalone and should not involve any external logic if not absolutely necessary. That's because you want the corpus of the tests to be easy to read and understandable at first sight.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-937318028
I'm not sold on the "Avoid nesting when you're testing" point. I personally like the abstractions and groupings and dryer code that comes from that long-used pattern. If most other people are really in favor of this new pattern, then that's cool, but it's not my first choice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-937319674
Can we add more explanation to these points so that people reading in the future also understand the why:
>Usage of waitFor
Do not use it for elements that can be queried with find
It should only have one assertion at a time
Can we instead of discouraging people from using Cypress just relay the difference between end-to-end and integration tests? We still need people to continue to write end to end tests. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-928009954
### RTL
> https://kentcdodds.com/blog/avoid-nesting-when-youre-testing
+1
> Our test files use the test suffix. We should migrate those that use the spec suffix
+1
#### Priorities
I think priorities should be remarked as it is a common mistake:
https://testing-library.com/docs/queries/about/#priority
#### Using `act` unnecessarily
> render and fireEvent are already wrapped in act
https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning
#### Using `*ByRole`
By using the `name` option we can point to the items by their accessible name.
`screen.getByRole('button', {name: /hello world/i})`
Roles reference: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles
#### Prefer `userEvent` over `fireEvent`
The user-event library better represents the actual user actions.
@testing-library/user-event
#### Usage of `waitFor`
Do not use it for elements that can be queried with `find`
It should only have one assertion at a time
Do not perform side-effects actions inside `waitFor`. Use it only for assertions
#### Materials:
https://kentcdodds.com/blog/common-mistakes-with-react-testing-library
.....to be continued
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
michael-s-molina edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-949605378
> Hi! Sorry for chiming in while this is being voted on, but I think it's important to add that we should make sure not to change the codebase in order to fix a test. The only changes that should be added to code regarding tests should be adding a test-id if absolutely necessary.
When we use TDD we are always changing the codebase in order to fix a test. I'm assuming you're talking about specific types of changes that should be avoided. To make this distinction clearer, can you give an example of a change in our codebase that is currently being made to fix a test?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho closed issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho closed issue #16792:
URL: https://github.com/apache/superset/issues/16792
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
michael-s-molina edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-938673521
> I'm not sold on the "Avoid nesting when you're testing" point. I personally like the abstractions and groupings and dryer code that comes from that long-used pattern. If most other people are really in favor of this new pattern, then that's cool, but it's not my first choice.
No problem! I personally prefer to avoid `describe` because of the points described in Kent C. Dodds article but I'm also fine if the majority chooses to use it.
> Can we add more explanation to these points so that people reading in the future also understand the why:
Yes! Will do.
> Can we instead of discouraging people from using Cypress just relay the difference between end-to-end and integration tests? We still need people to continue to write end to end tests. :)
I was reading again and you're right! We can make it more clear that we're favoring RTL for integration tests when possible but Cypress remains essential for e2e tests. Nice catch!
> when does the afterEach not run? I think it's better to clean up after yourself than after someone else. :)
@geido Can you detail this?
> Another common antipattern is that people will add classes or ids just for testing. I know we're talking about using roles and test-ids, etc, but do you think we can explicitly discourage adding anything into code for testing except for a test-id?
I think so. Do you have a sentence in mind to make it more clear? You can reply here or edit the issue's description directly 😉
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-932237116
@geido @eschutho @lyndsiWilliams I compiled the contributions so far, added some more, and mounted the first version in the PR's description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
etr2460 commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-932375790
Oh sorry, one more thing (but not blocking):
Under cypress you have:
>Prefer data-test attributes to isolate selectors from eventual CSS, JS or text content changes.
Isn't that the opposite as what we recommend for RTL? If it's best practices, then I guess it's fine. But maybe we should further call out why we have 2 different recommendations?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-932401384
> Top level comment, do we want to summarize a list of Dos and Don'ts like Evan did for the emotion guidelines at the top? Not sure if that pattern always works
That was my initial thought also but I preferred to add more context to each DOs and Don'ts because some are not very clear without the context, e.g. "Do not use Cypress". But it's following the Do's and Don't approach:
Dos:
- Write simple, standalone tests
- Avoid nesting when you're testing
- Keep accessibility in mind when writing your tests
...
Don'ts:
- No warnings!
- Don't use act unnecessarily
- Do not use Cypress :)
...
> One other comment, if you could link an example test file or two to the guidelines (ideally a direct link to the current sha so it never changes) that could be helpful. Sometimes showing works better than telling (and honestly, i love being able to copy a test file as a template for the new one i'm making)
Great suggestion. Will do.
> Isn't that the opposite as what we recommend for RTL? If it's best practices, then I guess it's fine. But maybe we should further call out why we have 2 different recommendations?
Wow! Nice catch. I do agree with you. We can query for elements in Cypress using accessible attributes like `role`. I was reading the [Cypress Best Practices](https://docs.cypress.io/guides/references/best-practices#Selecting-Elements) and it seems their main concern is to use selectors that don't change over time. So I think we'll be fine following RTL query [Priority](https://testing-library.com/docs/queries/about/#priority) and only use the `data-test` as a last resort.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-930146850
### Cypress
I am not much of a Cypress expert and I am taking what I think are the most relevant points for us from the official documentation, as well as adding some thoughts from my times working with it.
#### Do not use Cypress :)
Do not use Cypress when RTL can do it better and faster. Many of the Cypress tests that we have right now can be ported into RLT that is much faster and gives a more immediate feedback. Cypress should be used mainly for processes from A to B, B to C, etc. replicating the user experience, positive and negative flows.
#### Isolated and standalone tests
Tests should never rely on other tests to pass. This might be hard when a single user is used for testing as data will be stored into the database. At every new test we should reset the database. This discussion goes a bit out of the scope of the testing guidelines but might be worth pursuing.
#### Cleaning state
Cleaning the state of the application, such as resetting the db, or in general any state that might affect consequent tests, should always be done in the `beforeEach` hook and never in the `afterEach` one as the `beforeEach` is guaranteed to run, while the test might never reach the point to run the `afterEach`hook.
#### Unnecessary use of `cy.wait`
- Unnecessary when using `cy.request()` as it will resolve when a response is received from the server
- Unnecessary when using `cy.visit()` as it resolves only when the page fires the `load` event
- Unnecessary when using `cy.get()`. When the selector should wait for a request to happen, aliases would come handy. For example:
```
cy.intercept('GET', '/users', [{ name: 'Maggy' }, { name: 'Joan' }]).as(
'getUsers'
)
cy.get('#fetch').click()
cy.wait('@getUsers') // <--- wait explicitly for this route to finish
cy.get('table tr').should('have.length', 2)
```
#### Resilience
Prefer `data-*` attributes to isolate selectors from eventual CSS, JS or text content changes. Creating dedicated data selectors specifically for Cypress might increase the boilerplate a bit but comes with the advantage of being super resilient while making it apparent that the code is being tested. For example:
`cy.get('[data-cy=submit]').click()`
Alternatively using text might work but it is not as resilient. For example:
`cy.contains('Submit').click()`
#### Materials:
https://docs.cypress.io/guides/references/best-practices
https://www.youtube.com/watch?v=5XQOK0v_YRE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-928052969
These are great points @geido. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
eschutho edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-937318028
This is great @michael-s-molina!
I'm not sold on the "Avoid nesting when you're testing" point. I personally like the abstractions and groupings and dryer code that comes from that long-used pattern. If most other people are really in favor of this new pattern, then that's cool, but it's not my first choice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] geido edited a comment on issue #16792: [Proposal] Superset Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
geido edited a comment on issue #16792:
URL: https://github.com/apache/superset/issues/16792#issuecomment-928009954
### RTL
> https://kentcdodds.com/blog/avoid-nesting-when-youre-testing
+1
> Our test files use the test suffix. We should migrate those that use the spec suffix
+1
#### Priorities
I think priorities should be remarked as it is a common mistake:
https://testing-library.com/docs/queries/about/#priority
#### getByTestId
This is not viewable by the user and should only be used when the element isn't accessible in any other way.
#### Using `act` unnecessarily
`render` and `fireEvent` are already wrapped in `act`, so wrapping them in `act` again is a common mistake. Some solutions to the warning related to `act` might be found here: https://kentcdodds.com/blog/fix-the-not-wrapped-in-act-warning
#### Using `*ByRole`
By using the `name` option we can point to the items by their accessible name. For example:
`screen.getByRole('button', {name: /hello world/i})`
#### Prefer `userEvent` over `fireEvent`
The user-event library better represents the actual user actions.
`@testing-library/user-event`
#### Usage of `waitFor`
- Do not use it for elements that can be queried with `find`
- It should only have one assertion at a time
- Do not perform side-effects actions inside `waitFor`. Use it only for assertions
#### Materials:
https://kentcdodds.com/blog/common-mistakes-with-react-testing-library
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles
.....to be continued
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org