-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add logo #29
Conversation
jest.config.js
Outdated
@@ -9,6 +9,11 @@ module.exports = { | |||
transform: { | |||
"^.+\\.(ts|tsx)$": "ts-jest", | |||
}, | |||
moduleNameMapper: { |
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.
@dipeshwalia is it the only way to mock png files? should I do that any way?
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 we need this. even why we need jest.config.js ?, you can use assetsTransformer for jest
Package.json
"jest": {
"moduleNameMapper": {
"\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$": "<rootDir>/src/utils/assetsTransformer.js",
"\\.(css|less)$": "<rootDir>/src/utils/assetsTransformer.js"
}
}
assetsTransformer:
jestjs/jest#2663 (comment)
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 should me merge jest.config into package.json?
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.
Because it's mis leading.
React scripts provide us basic jest configuration and not every jest configuration will be entertain by react scripts.
https://create-react-app.dev/docs/running-tests/#configuration
@@ -13,6 +13,20 @@ exports[`Bootcamp renders as expected 1`] = ` | |||
<div | |||
className="MuiToolbar-root MuiToolbar-regular MuiToolbar-gutters" |
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.
I don't see value with snapshot 🌊
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 is that?
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.
Testing = confidence in app.
Snapshot doesn't bring any.
Codecov Report
@@ Coverage Diff @@
## master #29 +/- ##
==========================================
+ Coverage 97.21% 97.31% +0.10%
==========================================
Files 23 24 +1
Lines 251 261 +10
Branches 8 8
==========================================
+ Hits 244 254 +10
Misses 7 7
Continue to review full report at Codecov.
|
className?: string | undefined | ||
} | ||
|
||
const getLogoPath = () => `${process.env.PUBLIC_URL}/static/images/logo.png` |
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.
You ok with this Boss @dipeshwalia ?
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.
this is ok for now, but its good to avoid any env on ui.
Later on we can have configuration service to feed those value to Ui.
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.
I'm gonna add an issue for that
Removed a mock file
Added assetsTransformer.js
Updated jest.config
Updated Bootcamp snapshot