Closed davidlag0 closed 1 year ago
Looks good to me
The only problem that I see is the mixture of the ES6 and CommonJS modules. Is something broken with ES6? It is better to migrate fully to the ES6.
So the var
usage is unnecessary with the ES6 migration most of the time. It is better to use the let
and const
.
There you go, should be better now and all tests still pass. Would you mind keeping issue #135 opened until all tests have been built for the frontend?
There you go, should be better now and all tests still pass. Would you mind keeping issue https://github.com/dec0dOS/zero-ui/issues/135 opened until all tests have been built for the frontend?
Yes, certainly
Are you going to open the new PR with other frontend tests so I should merge it now, or we could just change the current PR title?
Yes! It will take me time to create all tests. It went somewhat quickly for the 2 components included in this PR because they were somewhat small but with more complex ones, it could take me weeks to cover them well. I already updated the PR title with the additional tests I added this morning and if you agree, this PR could be merged and I would open additional PR's as I move along with more tests until issue #136 is fully resolved.
I think that it's better to have an import path like components/HomeLoggedOut
, but that's is more a subjective preference question, so the decision is yours.
To be honest, I thought about it, I hesitated and when I saw the other imports did not have anything before them and were names like HomeLoggedOut
without any other part behind that, I went ahead with the fix. Give me a few minutes and I can change it with components/HomeLoggedOut
as you propose.
Is there any additional change you would like to see for this PR?
Seems to be something is wrong now.
❯ git clone https://github.com/davidlag0/zero-ui
Cloning into 'zero-ui'...
remote: Enumerating objects: 1406, done.
remote: Counting objects: 100% (479/479), done.
remote: Compressing objects: 100% (214/214), done.
remote: Total 1406 (delta 271), reused 385 (delta 242), pack-reused 927
Receiving objects: 100% (1406/1406), 6.21 MiB | 7.85 MiB/s, done.
Resolving deltas: 100% (751/751), done.
❯ cd zero-ui
❯ git checkout test/front_end_tests
branch 'test/front_end_tests' set up to track 'origin/test/front_end_tests'.
Switched to a new branch 'test/front_end_tests'
❯ yarn install
➤ YN0000: ┌ Resolution step
➤ YN0002: │ frontend@workspace:frontend doesn't provide postcss (p3d569), requested by jest-transform-css
➤ YN0000: │ Some peer dependencies are incorrectly met; run yarn explain peer-requirements <hash> for details, where <hash> is the six-letter p-prefixed code
➤ YN0000: └ Completed in 0s 216ms
➤ YN0000: ┌ Fetch step
➤ YN0000: └ Completed in 2s 177ms
➤ YN0000: ┌ Link step
➤ YN0007: │ nodemon@npm:2.0.19 must be built because it never has been before or the last one failed
➤ YN0007: │ styled-components@npm:5.3.5 [129c1] must be built because it never has been before or the last one failed
➤ YN0007: │ fsevents@patch:fsevents@npm%3A1.2.13#~builtin<compat/fsevents>::version=1.2.13&hash=d11327 must be built because it never has been before or the last one failed
➤ YN0007: │ core-js@npm:3.25.0 must be built because it never has been before or the last one failed
➤ YN0007: │ es5-ext@npm:0.10.62 must be built because it never has been before or the last one failed
➤ YN0007: │ core-js-pure@npm:3.25.0 must be built because it never has been before or the last one failed
➤ YN0007: │ ejs@npm:2.7.4 must be built because it never has been before or the last one failed
➤ YN0007: │ core-js@npm:2.6.12 must be built because it never has been before or the last one failed
➤ YN0007: │ zero-ui@workspace:. must be built because it never has been before or the last one failed
➤ YN0000: └ Completed in 10s 294ms
➤ YN0000: Done with warnings in 12s 873ms
❯ cd frontend
❯ yarn test:unit
PASS __tests__/unit/NetworkHeader.test.jsx
PASS __tests__/unit/NetworkHeader.snapshot.jsx
PASS __tests__/unit/HomeLoggedOut.snapshot.jsx
● Console
console.error
Warning: Failed prop type: The prop `justify` of `ForwardRef(Grid)` is deprecated. Use `justifyContent` instead, the prop was renamed.
at Grid (/private/tmp/zero-ui/node_modules/@material-ui/core/Grid/Grid.js:243:35)
at WithStyles (/private/tmp/zero-ui/node_modules/@material-ui/styles/withStyles/withStyles.js:67:31)
at HomeLoggedOut (/private/tmp/zero-ui/frontend/src/components/HomeLoggedOut/HomeLoggedOut.jsx:9:42)
3 |
4 | it("renders HomeLoggedOut unchanged", () => {
> 5 | const { container } = render(<HomeLoggedOut />);
| ^
6 | expect(container).toMatchSnapshot();
7 | });
8 |
at printWarning (../node_modules/react/cjs/react.development.js:220:30)
at error (../node_modules/react/cjs/react.development.js:196:5)
at checkPropTypes (../node_modules/react/cjs/react.development.js:1935:11)
at validatePropTypes (../node_modules/react/cjs/react.development.js:2136:7)
at Object.createElementWithValidation [as createElement] (../node_modules/react/cjs/react.development.js:2240:5)
at WithStyles (../node_modules/@material-ui/styles/withStyles/withStyles.js:97:42)
at renderWithHooks (../node_modules/react-dom/cjs/react-dom.development.js:14985:18)
at updateForwardRef (../node_modules/react-dom/cjs/react-dom.development.js:17044:20)
at beginWork (../node_modules/react-dom/cjs/react-dom.development.js:19098:16)
at beginWork$1 (../node_modules/react-dom/cjs/react-dom.development.js:23940:14)
at performUnitOfWork (../node_modules/react-dom/cjs/react-dom.development.js:22779:12)
at workLoopSync (../node_modules/react-dom/cjs/react-dom.development.js:22707:5)
at renderRootSync (../node_modules/react-dom/cjs/react-dom.development.js:22670:7)
at performSyncWorkOnRoot (../node_modules/react-dom/cjs/react-dom.development.js:22293:18)
at scheduleUpdateOnFiber (../node_modules/react-dom/cjs/react-dom.development.js:21881:7)
at updateContainer (../node_modules/react-dom/cjs/react-dom.development.js:25482:3)
at ../node_modules/react-dom/cjs/react-dom.development.js:26021:7
at unbatchedUpdates (../node_modules/react-dom/cjs/react-dom.development.js:22431:12)
at legacyRenderSubtreeIntoContainer (../node_modules/react-dom/cjs/react-dom.development.js:26020:5)
at Object.render (../node_modules/react-dom/cjs/react-dom.development.js:26103:10)
at ../node_modules/@testing-library/react/dist/pure.js:101:25
at batchedUpdates$1 (../node_modules/react-dom/cjs/react-dom.development.js:22380:12)
at act (../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1042:14)
at render (../node_modules/@testing-library/react/dist/pure.js:97:26)
at Object.<anonymous> (__tests__/unit/HomeLoggedOut.snapshot.jsx:5:31)
PASS __tests__/unit/HomeLoggedOut.test.jsx
● Console
console.error
Warning: Failed prop type: The prop `justify` of `ForwardRef(Grid)` is deprecated. Use `justifyContent` instead, the prop was renamed.
at Grid (/private/tmp/zero-ui/node_modules/@material-ui/core/Grid/Grid.js:243:35)
at WithStyles (/private/tmp/zero-ui/node_modules/@material-ui/styles/withStyles/withStyles.js:67:31)
at HomeLoggedOut (/private/tmp/zero-ui/frontend/src/components/HomeLoggedOut/HomeLoggedOut.jsx:9:42)
at Router (/private/tmp/zero-ui/node_modules/react-router/cjs/react-router.js:95:30)
16 | mock.onGet("/auth/login").reply(200, { enabled: true });
17 |
> 18 | render(
| ^
19 | <Router history={history}>
20 | <HomeLoggedOut />
21 | </Router>
at printWarning (../node_modules/react/cjs/react.development.js:220:30)
at error (../node_modules/react/cjs/react.development.js:196:5)
at checkPropTypes (../node_modules/react/cjs/react.development.js:1935:11)
at validatePropTypes (../node_modules/react/cjs/react.development.js:2136:7)
at Object.createElementWithValidation [as createElement] (../node_modules/react/cjs/react.development.js:2240:5)
at WithStyles (../node_modules/@material-ui/styles/withStyles/withStyles.js:97:42)
at renderWithHooks (../node_modules/react-dom/cjs/react-dom.development.js:14985:18)
at updateForwardRef (../node_modules/react-dom/cjs/react-dom.development.js:17044:20)
at beginWork (../node_modules/react-dom/cjs/react-dom.development.js:19098:16)
at beginWork$1 (../node_modules/react-dom/cjs/react-dom.development.js:23940:14)
at performUnitOfWork (../node_modules/react-dom/cjs/react-dom.development.js:22779:12)
at workLoopSync (../node_modules/react-dom/cjs/react-dom.development.js:22707:5)
at renderRootSync (../node_modules/react-dom/cjs/react-dom.development.js:22670:7)
at performSyncWorkOnRoot (../node_modules/react-dom/cjs/react-dom.development.js:22293:18)
at scheduleUpdateOnFiber (../node_modules/react-dom/cjs/react-dom.development.js:21881:7)
at updateContainer (../node_modules/react-dom/cjs/react-dom.development.js:25482:3)
at ../node_modules/react-dom/cjs/react-dom.development.js:26021:7
at unbatchedUpdates (../node_modules/react-dom/cjs/react-dom.development.js:22431:12)
at legacyRenderSubtreeIntoContainer (../node_modules/react-dom/cjs/react-dom.development.js:26020:5)
at Object.render (../node_modules/react-dom/cjs/react-dom.development.js:26103:10)
at ../node_modules/@testing-library/react/dist/pure.js:101:25
at batchedUpdates$1 (../node_modules/react-dom/cjs/react-dom.development.js:22380:12)
at act (../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1042:14)
at render (../node_modules/@testing-library/react/dist/pure.js:97:26)
at Object.<anonymous> (__tests__/unit/HomeLoggedOut.test.jsx:18:11)
--------------------|---------|----------|---------|---------|-------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
--------------------|---------|----------|---------|---------|-------------------
All files | 100 | 100 | 100 | 100 |
HomeLoggedOut | 100 | 100 | 100 | 100 |
HomeLoggedOut.jsx | 100 | 100 | 100 | 100 |
index.jsx | 0 | 0 | 0 | 0 |
NetworkHeader | 100 | 100 | 100 | 100 |
NetworkHeader.jsx | 100 | 100 | 100 | 100 |
index.jsx | 0 | 0 | 0 | 0 |
--------------------|---------|----------|---------|---------|-------------------
Test Suites: 4 passed, 4 total
Tests: 6 passed, 6 total
Snapshots: 2 passed, 2 total
Time: 1.801 s
Ran all test suites matching /unit/i.
Seems to be something is wrong now.
❯ git clone https://github.com/davidlag0/zero-ui Cloning into 'zero-ui'... remote: Enumerating objects: 1406, done. remote: Counting objects: 100% (479/479), done. remote: Compressing objects: 100% (214/214), done. remote: Total 1406 (delta 271), reused 385 (delta 242), pack-reused 927 Receiving objects: 100% (1406/1406), 6.21 MiB | 7.85 MiB/s, done. Resolving deltas: 100% (751/751), done. ❯ cd zero-ui ❯ git checkout test/front_end_tests branch 'test/front_end_tests' set up to track 'origin/test/front_end_tests'. Switched to a new branch 'test/front_end_tests' ❯ yarn install ➤ YN0000: ┌ Resolution step ➤ YN0002: │ frontend@workspace:frontend doesn't provide postcss (p3d569), requested by jest-transform-css ➤ YN0000: │ Some peer dependencies are incorrectly met; run yarn explain peer-requirements <hash> for details, where <hash> is the six-letter p-prefixed code ➤ YN0000: └ Completed in 0s 216ms ➤ YN0000: ┌ Fetch step ➤ YN0000: └ Completed in 2s 177ms ➤ YN0000: ┌ Link step ➤ YN0007: │ nodemon@npm:2.0.19 must be built because it never has been before or the last one failed ➤ YN0007: │ styled-components@npm:5.3.5 [129c1] must be built because it never has been before or the last one failed ➤ YN0007: │ fsevents@patch:fsevents@npm%3A1.2.13#~builtin<compat/fsevents>::version=1.2.13&hash=d11327 must be built because it never has been before or the last one failed ➤ YN0007: │ core-js@npm:3.25.0 must be built because it never has been before or the last one failed ➤ YN0007: │ es5-ext@npm:0.10.62 must be built because it never has been before or the last one failed ➤ YN0007: │ core-js-pure@npm:3.25.0 must be built because it never has been before or the last one failed ➤ YN0007: │ ejs@npm:2.7.4 must be built because it never has been before or the last one failed ➤ YN0007: │ core-js@npm:2.6.12 must be built because it never has been before or the last one failed ➤ YN0007: │ zero-ui@workspace:. must be built because it never has been before or the last one failed ➤ YN0000: └ Completed in 10s 294ms ➤ YN0000: Done with warnings in 12s 873ms ❯ cd frontend ❯ yarn test:unit PASS __tests__/unit/NetworkHeader.test.jsx PASS __tests__/unit/NetworkHeader.snapshot.jsx PASS __tests__/unit/HomeLoggedOut.snapshot.jsx ● Console console.error Warning: Failed prop type: The prop `justify` of `ForwardRef(Grid)` is deprecated. Use `justifyContent` instead, the prop was renamed. at Grid (/private/tmp/zero-ui/node_modules/@material-ui/core/Grid/Grid.js:243:35) at WithStyles (/private/tmp/zero-ui/node_modules/@material-ui/styles/withStyles/withStyles.js:67:31) at HomeLoggedOut (/private/tmp/zero-ui/frontend/src/components/HomeLoggedOut/HomeLoggedOut.jsx:9:42) 3 | 4 | it("renders HomeLoggedOut unchanged", () => { > 5 | const { container } = render(<HomeLoggedOut />); | ^ 6 | expect(container).toMatchSnapshot(); 7 | }); 8 | at printWarning (../node_modules/react/cjs/react.development.js:220:30) at error (../node_modules/react/cjs/react.development.js:196:5) at checkPropTypes (../node_modules/react/cjs/react.development.js:1935:11) at validatePropTypes (../node_modules/react/cjs/react.development.js:2136:7) at Object.createElementWithValidation [as createElement] (../node_modules/react/cjs/react.development.js:2240:5) at WithStyles (../node_modules/@material-ui/styles/withStyles/withStyles.js:97:42) at renderWithHooks (../node_modules/react-dom/cjs/react-dom.development.js:14985:18) at updateForwardRef (../node_modules/react-dom/cjs/react-dom.development.js:17044:20) at beginWork (../node_modules/react-dom/cjs/react-dom.development.js:19098:16) at beginWork$1 (../node_modules/react-dom/cjs/react-dom.development.js:23940:14) at performUnitOfWork (../node_modules/react-dom/cjs/react-dom.development.js:22779:12) at workLoopSync (../node_modules/react-dom/cjs/react-dom.development.js:22707:5) at renderRootSync (../node_modules/react-dom/cjs/react-dom.development.js:22670:7) at performSyncWorkOnRoot (../node_modules/react-dom/cjs/react-dom.development.js:22293:18) at scheduleUpdateOnFiber (../node_modules/react-dom/cjs/react-dom.development.js:21881:7) at updateContainer (../node_modules/react-dom/cjs/react-dom.development.js:25482:3) at ../node_modules/react-dom/cjs/react-dom.development.js:26021:7 at unbatchedUpdates (../node_modules/react-dom/cjs/react-dom.development.js:22431:12) at legacyRenderSubtreeIntoContainer (../node_modules/react-dom/cjs/react-dom.development.js:26020:5) at Object.render (../node_modules/react-dom/cjs/react-dom.development.js:26103:10) at ../node_modules/@testing-library/react/dist/pure.js:101:25 at batchedUpdates$1 (../node_modules/react-dom/cjs/react-dom.development.js:22380:12) at act (../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1042:14) at render (../node_modules/@testing-library/react/dist/pure.js:97:26) at Object.<anonymous> (__tests__/unit/HomeLoggedOut.snapshot.jsx:5:31) PASS __tests__/unit/HomeLoggedOut.test.jsx ● Console console.error Warning: Failed prop type: The prop `justify` of `ForwardRef(Grid)` is deprecated. Use `justifyContent` instead, the prop was renamed. at Grid (/private/tmp/zero-ui/node_modules/@material-ui/core/Grid/Grid.js:243:35) at WithStyles (/private/tmp/zero-ui/node_modules/@material-ui/styles/withStyles/withStyles.js:67:31) at HomeLoggedOut (/private/tmp/zero-ui/frontend/src/components/HomeLoggedOut/HomeLoggedOut.jsx:9:42) at Router (/private/tmp/zero-ui/node_modules/react-router/cjs/react-router.js:95:30) 16 | mock.onGet("/auth/login").reply(200, { enabled: true }); 17 | > 18 | render( | ^ 19 | <Router history={history}> 20 | <HomeLoggedOut /> 21 | </Router> at printWarning (../node_modules/react/cjs/react.development.js:220:30) at error (../node_modules/react/cjs/react.development.js:196:5) at checkPropTypes (../node_modules/react/cjs/react.development.js:1935:11) at validatePropTypes (../node_modules/react/cjs/react.development.js:2136:7) at Object.createElementWithValidation [as createElement] (../node_modules/react/cjs/react.development.js:2240:5) at WithStyles (../node_modules/@material-ui/styles/withStyles/withStyles.js:97:42) at renderWithHooks (../node_modules/react-dom/cjs/react-dom.development.js:14985:18) at updateForwardRef (../node_modules/react-dom/cjs/react-dom.development.js:17044:20) at beginWork (../node_modules/react-dom/cjs/react-dom.development.js:19098:16) at beginWork$1 (../node_modules/react-dom/cjs/react-dom.development.js:23940:14) at performUnitOfWork (../node_modules/react-dom/cjs/react-dom.development.js:22779:12) at workLoopSync (../node_modules/react-dom/cjs/react-dom.development.js:22707:5) at renderRootSync (../node_modules/react-dom/cjs/react-dom.development.js:22670:7) at performSyncWorkOnRoot (../node_modules/react-dom/cjs/react-dom.development.js:22293:18) at scheduleUpdateOnFiber (../node_modules/react-dom/cjs/react-dom.development.js:21881:7) at updateContainer (../node_modules/react-dom/cjs/react-dom.development.js:25482:3) at ../node_modules/react-dom/cjs/react-dom.development.js:26021:7 at unbatchedUpdates (../node_modules/react-dom/cjs/react-dom.development.js:22431:12) at legacyRenderSubtreeIntoContainer (../node_modules/react-dom/cjs/react-dom.development.js:26020:5) at Object.render (../node_modules/react-dom/cjs/react-dom.development.js:26103:10) at ../node_modules/@testing-library/react/dist/pure.js:101:25 at batchedUpdates$1 (../node_modules/react-dom/cjs/react-dom.development.js:22380:12) at act (../node_modules/react-dom/cjs/react-dom-test-utils.development.js:1042:14) at render (../node_modules/@testing-library/react/dist/pure.js:97:26) at Object.<anonymous> (__tests__/unit/HomeLoggedOut.test.jsx:18:11) --------------------|---------|----------|---------|---------|------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s --------------------|---------|----------|---------|---------|------------------- All files | 100 | 100 | 100 | 100 | HomeLoggedOut | 100 | 100 | 100 | 100 | HomeLoggedOut.jsx | 100 | 100 | 100 | 100 | index.jsx | 0 | 0 | 0 | 0 | NetworkHeader | 100 | 100 | 100 | 100 | NetworkHeader.jsx | 100 | 100 | 100 | 100 | index.jsx | 0 | 0 | 0 | 0 | --------------------|---------|----------|---------|---------|------------------- Test Suites: 4 passed, 4 total Tests: 6 passed, 6 total Snapshots: 2 passed, 2 total Time: 1.801 s Ran all test suites matching /unit/i.
I came across this, but it is technically unrelated to the PR. I can make the changes anyways if you want to see the errors gone. I did a successful test change with HomeLoggedOut.jsx
last week: we just need to replace justify
with justifyContent
Ok, that's fine, I could merge it now.
Thank you very much! I'll proceed to add more tests!
Pull Request type
Please check the type of change your PR introduces:
What is the current behavior?
There are currently no automated tests with the codebase. This pull request is meant to add such support for the frontend only for now using Jest and other related tools and to provide the tests for the first component: HomeLoggedOut.
Issue Number: #135
What is the new behavior?
Automated tests will now be supported for the frontend. More tests are required to fully cover the codebase but this is a starting point to eventually enable GitHub's dependabot, automated tests with GitHub Actions, etc.
Does this introduce a breaking change?
The changes are mainly related to development-related functionalities.
package.json
is updated to add the required additional dev dependencies and so isyarn.lock
to reflect these changes.Other information
Don't hesitate to ask me any question you might have about the change.