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/11/05 17:42:56 UTC
[GitHub] [superset] eschutho opened a new issue #17359: [Proposal] Edit to Frontend Testing Guidelines
eschutho opened a new issue #17359:
URL: https://github.com/apache/superset/issues/17359
This is a proposal to edit the frontend testing guidelines from:
```
Avoid nesting when you're testing
Avoid the use of describe blocks in favor of inlined tests. If your tests start to grow and you feel the need to group tests, prefer to break them into multiple test files. Check this [awesome article](https://kentcdodds.com/blog/avoid-nesting-when-youre-testing) written by Kent C. Dodds about this topic.
```
to
Avoid unnecessary abstractions
Even though there may be a lot of duplication in tests because you are performing the same actions many times, avoid too many abstractions and drying up code if it means that the tests become unreadable. If almost each line in your test is calling a function defined in a beforeEach block for example, consider its readability and bringing that code back into the test, especially if it's a short block of code that is repeated.
I feel that the sentiment of KCD's article is to avoid too much abstraction that would make tests difficult to read. There are still benefits to nesting in describe for the purpose of keeping scenarios together, especially since jest strings together describe tags. i.e.,
```
describe('myComponent', () => {
describe('when user is logged in', () => {
describe('and they do not have a photo', () => {
it('should do something')
});
describe('when the user is not logged in', () => {
it('should do something')
});
});
describe('when the user is not logged in', () => {
it('should do something')
});
});
```
This writes out the test case like: 'myComponent when user is logged in and they do not have a photo'
so it's very convenient to make sure that you're covering a binary set of cases and still have a readable output.
--
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 #17359: [Proposal] Edit to Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #17359:
URL: https://github.com/apache/superset/issues/17359#issuecomment-962116333
Nesting also provides what I'd call an "inversion" problem... for example if tests are structured like so:
- When user is logged in
- When button is clicked
- Expect X happens
- When a user is logged out
- When button is clicked
- Expect Y happens
When the outer "logged in" and "logged out" blocks get crazy long, a developer might want to invert the structure to be something like:
- When button is clicked
- When user is logged in
- Expect X happens
- When a user is logged out
- Expect Y happens
If there were hundreds of tests, making that change would be painful. If atomic tests were in folders/files, it might not be AS painful to restructure. ¯\\\_(ツ)_/¯
Really though, I think both patterns are fine, and it's ultimately a question of having some sane threshold of _when_ to break it out by file.
--
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 #17359: [Proposal] Edit to Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #17359:
URL: https://github.com/apache/superset/issues/17359#issuecomment-962108229
--
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 #17359: [Proposal] Edit to Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #17359:
URL: https://github.com/apache/superset/issues/17359#issuecomment-962108229
I think the truth here (and the difficulty of writing it up as a guideline) is finding a balance somewhere in the middle. I think the "break it up into files" approach is to (a) keep files from being so huge they're hard to maintain, and (b) keeping areas of functionality/intent/assertion orgainized in an approachable way.
Nested describes do solve some useful concerns as Elizabeth states, however, in terms of organizing thoughts or assertions. I don't think there should be HUGE blocks, but it makes sense to organize things in a way such that if you break one test, you might want to think through the sibling/parent assertions/blocks.
--
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 #17359: [Proposal] Edit to Frontend Testing Guidelines
Posted by GitBox <gi...@apache.org>.
rusackas commented on issue #17359:
URL: https://github.com/apache/superset/issues/17359#issuecomment-962211939
Continuing to ramble... just some notions of what we discussed in a chat. Both patterns (nesting describes, breaking out into discrete files) make a lot of sense. We should capture these in `DO` and `DON'T` statements... things like
`DO` Use describe blocks to group small/reasonable sets of related tests, where editing one means you should probably revisit the others
`DON'T` let nested describes get so long that it's difficult to navigate the hierarchy. If things get long, break them out into files!
`DO` use separate files to organize and encapsulate tests of discrete areas/concerns
`DON'T` get too "atomic" with excessively numerous test files to where it's difficult to find a test or maintain a set of tests
`DON'T` use unnecessary abstractions - make sure it's possible for a new reader to easily understand the logic/process of a test.
... etc. You get the idea 😁 Thanks for bringing this conversation to light!
--
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