Skip to content
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

Unit test and documentation #20

Conversation

SyntaxXeror
Copy link
Contributor

Small update to fix some of the issues from issue #18 :

  • Add user instructions in docs/CLIENT.md
  • remove warning for react grid test

Omarley7 and others added 7 commits March 22, 2023 11:01
* DUGK-37 iframe test

* DUGK-26 Made tests work on iframe.
Test coverage added on `yarn test`.

---------

Co-authored-by: Asger <asger_bb@hotmail.com>
* Updated typescript and jest libaries.
Added types for react and react-dom.
Removed conflicting @jest/globals package
Removed all babel, jest is using ts-jest.

* Added import of jest-dom to jest.config.json.
It's no longer needed in tests.

* Build with no warnings and functional test.

Re-instaded babel needed for react-scripts to run.
Removed transform from jest config and eslint.

* Remved jest/globals reference from tests.
Slight modification to eslint.
* DUGK-32 Added mock for "ResizeObserver" to setup.
Don't know it's behavoir, use at own discretion.

* DUGK-32 Unit test for Dashboard with valid mocks

* DUGK-32 added back the documentation which have been deleted

---------

Co-authored-by: Omar <omarg@live.dk>
const { getByTitle } = render(<Iframe url={url} title={title} />);
const iframe = getByTitle(title);
expect(iframe).toHaveProperty('src', url);
it('renders an iframe element with the correct src and title', () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

const { getByTitle } = render(<Iframe url={url} title={title} />);
const iframe = getByTitle(title);
expect(iframe).toHaveProperty('width', '100%');
it('sets the iframe width to 100%', () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2023

Codecov Report

Merging #20 (9de72f6) into feature/distributed-demo (cbb8833) will increase coverage by 35.11%.
The diff coverage is 54.54%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@                      Coverage Diff                      @@
##           feature/distributed-demo      #20       +/-   ##
=============================================================
+ Coverage                          0   35.11%   +35.11%     
=============================================================
  Files                             0       25       +25     
  Lines                             0      262      +262     
=============================================================
+ Hits                              0       92       +92     
- Misses                            0      170      +170     
Flag Coverage Δ
client-browser-test ∅ <ø> (?)
client-unit-test 35.11% <54.54%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
client/src/AppProvider.tsx 0.00% <0.00%> (ø)
client/src/index.tsx 0.00% <0.00%> (ø)
client/src/page/Menu.tsx 0.00% <ø> (ø)
client/src/route/auth/Account.tsx 0.00% <0.00%> (ø)
client/src/page/Layout.tsx 100.00% <100.00%> (ø)
client/src/route/dashboard/Dashboard.tsx 100.00% <100.00%> (ø)
client/src/route/digitaltwins/DigitalTwins.tsx 100.00% <100.00%> (ø)
client/src/route/history/History.tsx 100.00% <100.00%> (ø)
client/src/route/history/Logs.tsx 100.00% <100.00%> (ø)
client/src/route/history/RecentRuns.tsx 100.00% <100.00%> (ø)
... and 3 more

... and 12 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Omarley7 and others added 4 commits March 24, 2023 10:34
Added husky, not configured yet.
* fix codeclimate in iframe test

* fixed another codeclimate issue
- Added unittest of workbench
expect(true);
});

it('renders Dashboard woth components', () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prasadtalasila So codeclimate is kind of dumb when it comes to test... Is it okay if we exclude test from codeclimate or how do we prevent this redunt code smell?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SyntaxXeror
Please check codeclimate config docs. If I am not mistaken, there must be a way to skip this problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prasadtalasila you already replyed to this early where you wrote directly from email, so this is fixed :)

expect(true);
});

it('renders components', () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar blocks of code found in 2 locations. Consider refactoring.

* DUGK-30 Forward unknown routes to backend
Move the layout to individual pages and cleanup the index.tsx

* DUGK-30 refactored theme to standalone appprovider.
Increases testability.

* DUGK-30 Started layout test pretty dirty.

* Dugk 32 test suite for page dashboard (#18)

* DUGK-32 Added mock for "ResizeObserver" to setup.
Don't know it's behavoir, use at own discretion.

* DUGK-32 Unit test for Dashboard with valid mocks

* DUGK-32 added back the documentation which have been deleted

---------

Co-authored-by: Omar <omarg@live.dk>

* Minor regex change for test environment

* Formatting with prettier.
Added husky, not configured yet.

* DUGK-30 Fixed duplaction in tests

* DUGK-30 added comment

* DUGK-30 added another comment

* DUGK-30 Moved comment

* DUGK-30 Actually solved the issue.
Moved grid item up to layout.

* DUGK-30 fixed iframe var

---------

Co-authored-by: Asger <71492620+SyntaxXeror@users.noreply.github.com>
Co-authored-by: Asger <asger_bb@hotmail.com>
@prasadtalasila
Copy link
Contributor

prasadtalasila commented Mar 27, 2023 via email

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR diff size of 5001 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR diff size of 5076 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR diff size of 5082 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR diff size of 5111 lines exceeds the maximum allowed for the inline comments feature.

* DUGK-34 Test suite created.

* DUGK-34 fixed climate issue

* DUGK-34 moved test to unittest dir
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR diff size of 5133 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR diff size of 5133 lines exceeds the maximum allowed for the inline comments feature.

* DUGK-40 Added husky
Configured pre-commit to run syntax and format.
Locked prettier version
Fixed AppProvider

* DUGK-41 Now running test on push.
Bypass with -y flag.

* DUGK-41 Removed failing bypass with '-y'
use `HUSKY=0 git push` to bypass

* excluded huske from codeclimate

* DUGK-41 Fixed build issue

* DUGK-41 skip husky on pipeline

---------

Co-authored-by: Asger <asger_bb@hotmail.com>
@SyntaxXeror SyntaxXeror reopened this Mar 27, 2023
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR diff size of 5161 lines exceeds the maximum allowed for the inline comments feature.

@SyntaxXeror
Copy link
Contributor Author

@prasadtalasila
This PR should address the rest of the issues:

  • Add unit tests and remove warning for react grid test
  • Forward unknown routes to backend
  • Move the layout to individual pages and cleanup the index.tsx
  • Fix codeclimate issues (except one placeholder component RecentRuns, which we will skip for now)
  • Add user instructions in docs/CLIENT.md (updated with instructions to change env-file)

We also added husky to our pipeline so it runs pre-commit and pre-push:

  • Commit: Will format all ts,tsx,css,scss files with Prettier and run eslint. If eslint fails, the commit won't go through. Override in terminal with git commit -m "message" --no-verify
  • Push: Will run all unittest with yarn jest . --coverage=false . If any tests fails, the push will fail. Override with in terminal HUSKY=0 git push.

@prasadtalasila
Copy link
Contributor

@prasadtalasila This PR should address the rest of the issues:

* Add unit tests and remove warning for react grid test

* Forward unknown routes to backend

* Move the layout to individual pages and cleanup the index.tsx

* Fix codeclimate issues (except one placeholder component RecentRuns, which we will skip for now)

* Add user instructions in docs/CLIENT.md (updated with instructions to change env-file)

We also added husky to our pipeline so it runs pre-commit and pre-push:

* Commit: Will format all ts,tsx,css,scss files with Prettier and run eslint. If eslint fails, the commit won't go through. Override in terminal with `git commit -m "message" --no-verify`

* Push: Will run all unittest with `yarn jest . --coverage=false` . If any tests fails, the push will fail.  Override with in terminal  `HUSKY=0 git push`.

This is great. typicode delivers high-quality packages. The json-server will be useful for testing.

I will check this PR by end of today. If everything is ok, will merge the same.

@prasadtalasila
Copy link
Contributor

  1. The test/unitTests/Library.test.tsx fails. Please see line-72 of build
  2. Add yarn format and yarn prepare to client/README.md, may be with a single line comment for each command. Does it make sense to put the commands in yarn prepare step in script/install.bash?

The git commit hook doesn't seem to work for me.

 prasad@prasad-Linux:DTaaS/client|omar-PR20⚡ ⇒  yarn prepare
yarn run v1.22.19
$ cd .. && husky install client/.husky
husky - Git hooks installed
Done in 0.20s.
prasad@prasad-Linux:DTaaS/client|omar-PR20⚡ ⇒  yarn syntax
yarn run v1.22.19
$ script/syntax.bash
Running eslint

DTaaS/client/src/AppProvider.tsx
  11:31  warning  'children' is defined but never used  @typescript-eslint/no-unused-vars

✖ 1 problem (0 errors, 1 warning)

Done in 9.89s.
prasad@prasad-Linux:DTaaS/client|omar-PR20⚡ ⇒  git add .      
prasad@prasad-Linux:DTaaS/client|omar-PR20⚡ ⇒  git commit -m "check commit hook"      
[omar-PR20 7202449] check commit hook
 7 files changed, 14 insertions(+), 15 deletions(-)

Am I doing something wrong? Same is the case with git push as well.

################ Running Jest ################                                                                                                                                         
 PASS  test/Layout.test.tsx (8.428 s)                                                                                                                                                  
  Layout component with one element
    ✓ renders without crashing (88 ms)
    ✓ has a main (70 ms)
    ✓ has menu and toolbar for spacing (12 ms)
    ✓ has footer (9 ms)
    ✓ has a single child (25 ms)
    ✓ has multiple children (2) (14 ms)

 PASS  test/unitTests/History/Logs.test.tsx
  Logs
    ✓ renders title (152 ms)
    ✓ renders body text (19 ms)
    ✓ renders "See more" link (8 ms)
    ✓ prevents default action when "See more" link is clicked (15 ms)

 FAIL  test/Iframe.test.tsx (10.785 s)
  Iframe
    ✕ renders an iframe element with the correct src and title (73 ms)
    ✓ sets the iframe width to 100% (20 ms)
    ✓ sets the iframe height to 100% if fullsize prop is not provided (18 ms)
  Iframe fullsize
    ✓ sets the iframe height to auto if fullsize prop is provided (24 ms)

  ● Iframe › renders an iframe element with the correct src and title

    expect(received).toBe(expected) // Object.is equality

    Expected: "https://example.doc/"
    Received: "https://example.com/"

      12 |
      13 |   it('renders an iframe element with the correct src and title', () => {
    > 14 |     expect(iframe.src).toBe('https://example.doc/');
         |                        ^
      15 |   });
      16 |
      17 |   it('sets the iframe width to 100%', () => {

      at Object.<anonymous> (test/Iframe.test.tsx:14:24)
....
 FAIL  test/unitTests/Library.test.tsx
  ● Test suite failed to run

    Cannot find module '../src/route/library/Components' from 'test/unitTests/Library.test.tsx'

      3 | import Library from 'route/library/Library';
      4 |
    > 5 | jest.mock('../src/route/library/Components', () => ({
        |      ^
      6 |   default: () => <div>LibComponents-mock</div>,
      7 | }));
      8 |

      at Resolver._throwModNotFoundError (node_modules/jest-cli/node_modules/jest-resolve/build/resolver.js:427:11)
      at Object.<anonymous> (test/unitTests/Library.test.tsx:5:6)

prasad@prasad-Linux:DTaaS/client|omar-PR20 ⇒  git push prasad omar-PR20
Enumerating objects: 295, done.
Counting objects: 100% (295/295), done.
Delta compression using up to 3 threads
Compressing objects: 100% (154/154), done.
Writing objects: 100% (233/233), 1.76 MiB | 527.00 KiB/s, done.
Total 233 (delta 137), reused 127 (delta 68)
remote: Resolving deltas: 100% (137/137), completed with 34 local objects.
remote: 
remote: Create a pull request for 'omar-PR20' on GitHub by visiting:
remote:      https://github.com/prasadtalasila/DTaaS/pull/new/omar-PR20
remote: 
To https://github.com/prasadtalasila/DTaaS.git
 * [new branch]      omar-PR20 -> omar-PR20

Everything else looks great.

Re-organizing unittests.
Fixed chart test.
Fixed husky install
Updated readme for husky and format script.
@Omarley7
Copy link
Contributor

Omarley7 commented Mar 28, 2023

@prasadtalasila

  1. I've organized the tests in folders and moved the test config to where appropriate. The test is now using an absolute path instead, which solved the issue.
    I had to suppress the console.error tests using rechart. The errors happen before the mock is initialized.

  2. Fixed the husky install script. Too many arguments. prepare will run right after yarn install as part of the yarn lifecycle automatically. Also if run from the install.bash script.
    Yarn 2 does require an edit in the package.json to post-install instead. I've updated the readme.

omar@fedora ~/g/D/client (development) [1]> yarn 
yarn install v1.22.19
[1/4] Resolving packages...
success Already up-to-date.
$ cd .. && husky install
husky - Git hooks installed
Done in 0.55s.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR diff size of 5244 lines exceeds the maximum allowed for the inline comments feature.

@codeclimate
Copy link

codeclimate bot commented Mar 28, 2023

Code Climate has analyzed commit 9de72f6 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@prasadtalasila prasadtalasila merged commit b90d574 into INTO-CPS-Association:feature/distributed-demo Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants