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
Jest add flexibility #7748
Jest add flexibility #7748
Conversation
…used the theme throughout
This pull request is automatically deployed with Now. |
…st-add-flexibility # Conflicts: # addons/jest/src/components/Panel.tsx
addons/jest/src/components/Panel.tsx
Outdated
const testsByType: Map<string, any> = new Map(); | ||
result.assertionResults.forEach((assertion: any) => { | ||
// using switch to allow for new types to be added | ||
switch (assertion.status) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just use assertion.status
instead of switch because the logic is the same in each branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my goodness, nice find. I could definitely use assertion.status instead. No need for the switch at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with possible exception of duplicate logic!
Also, what's the bug? |
@shilman switched it to maintenance. Do you think it would fall under that category? |
@CodeByAlex that was my thought 👍 |
What I did
Refactored the Jest code a bit from being an MVP to a more sustainable codebase. Built-in flexibility for adding new types of test status' that will help @fabiradi work on his bugfix/feature outlined here: #7740.
Sorted values coming into the status bar so that it will order the parts from smallest to largest
slightly improved performance by using fewer loops in render function
How to test
If your answer is yes to any of these, please make sure to include it in your PR.