WebApp update - Code quality #37

Omarley7 · 2023-04-10T13:58:02Z

Many issues from code climate have been addressed.
Added .gittattibute so WSL can run scripts on Windows machines with configuring line endings locally
Readme has been updated
React-tabs have been implemented instead of MUI tabs to reduce code and accommodate code climate issues.
Extended E2E test on menu to cover 100% of navigation

codecov-commenter · 2023-04-10T14:02:31Z

Codecov Report

Merging #37 (76c87f2) into feature/distributed-demo (c4abeaf) will increase coverage by 13.14%.
The diff coverage is 96.87%.

📣 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      #37       +/-   ##
=============================================================
+ Coverage                     49.07%   62.21%   +13.14%     
=============================================================
  Files                            31       31               
  Lines                           324      352       +28     
  Branches                          2        7        +5     
=============================================================
+ Hits                            159      219       +60     
+ Misses                          163      127       -36     
- Partials                          2        6        +4

Flag	Coverage Δ
client-browser-test	`∅ <ø> (∅)`
client-unit-test	`58.44% <96.87%> (+9.36%)`	⬆️
lib-microservice	`88.63% <ø> (?)`

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

Impacted Files	Coverage Δ
client/src/route/auth/AccountTabs.tsx	`0.00% <0.00%> (ø)`
client/src/components/Iframe.tsx	`100.00% <100.00%> (ø)`
client/src/components/tab/TabComponent.tsx	`100.00% <100.00%> (+100.00%)`	⬆️
...ent/src/components/tab/subcomponents/TabRender.tsx	`100.00% <100.00%> (ø)`
client/src/route/digitaltwins/DigitalTwins.tsx	`100.00% <100.00%> (ø)`
client/src/route/library/Library.tsx	`100.00% <100.00%> (ø)`
...nt/src/route/scenarioAnalysis/ScenarioAnalysis.tsx	`100.00% <100.00%> (ø)`
client/src/route/workbench/Workbench.tsx	`100.00% <100.00%> (ø)`

... and 4 files with indirect coverage changes

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

prasadtalasila

Other comments: I've missed these changes over time. The react-iframe npm package has been removed from src/components/Iframe.tsx. Doesn't it provide better functionality than the existing code?

.husky/pre-commit

prasadtalasila · 2023-04-11T08:12:53Z

client/test/e2e/Menu.test.ts

-    await page.goto('/sanalysis');
-    await page.locator('div[role="button"]:has-text("Library")').click();
-    await expect(page).toHaveURL('/library');
+  links.forEach((link) => {


This is a strange convention. Usually we put code inside test.describe() function. Now we are iterating on the test() function itself. Can you move the link.forEach() into test.describe()?

I'm pretty sure I read somewhere that it's also convention to loop outside of the test() function if needed (of course I failed to find where I read it).

In this case, we will have identical tests for each route. (Loop 1)

Each route navigates to every link to check if it works. (Loop 2)

Looping twice inside the test gave us many issues.

For now I've taken it out of the PR, and will look into it more in detail for the next PR.

prasadtalasila · 2023-04-11T08:13:22Z

client/test/e2e/Menu.test.ts

-    await page.goto('/library');
-    await page.locator('div[role="button"]:has-text("Digital Twins")').click();
-    await expect(page).toHaveURL('/digitaltwins');
+interface TestLink {


nice refactoring

prasadtalasila · 2023-04-11T08:18:47Z

client/src/components/tab/TabComponent.tsx

-import TabPanel, { a11yProps } from 'components/tab/subcomponents/TabPanel';
-import { useState, SyntheticEvent } from 'react';
+import { Paper } from '@mui/material';
+import TabRender from './subcomponents/TabRender';


There seems to be a circular dependency between TabComponents and TabRender.

Also as a convention, things in parent directories depend on sub-directories, except when they are in different packages, or in different parts of directory structure. For example, code in src/components/subcomponents/TabRender.tsx can depend on src/utility/sample.tsx but not on src/components/Components.tsx.

@DaloonOfDoom took a look at this and fixed it

prasadtalasila · 2023-04-11T08:37:13Z

At this moment, about 75% of the code quality issues are in client/. Is it possible to fix more code quality issues in this PR?

client/test/unitTests/__mocks__/page_mocks.tsx

prasadtalasila · 2023-04-20T07:56:54Z

@Omarley7, the scope of this PR seems to be expanding quite a bit. Can you please list the improvements and downgrades made in this PR so far?
If there are already significant improvements, let's merge this and push further work into next PR. Thanks.

Omarley7 · 2023-04-20T09:37:51Z

@prasadtalasila I was hoping to include the issue discussed in #42.
I'll make an update on the work in this PR and move some of the E2E test stuff to the next PR.

* DUGK-57 Updated readme instructions. Ultra clear build instructions. * DUGK-57 removed 'test'

* DUGK-61 Refactored /e2e/Menu.test.ts * DUGK-61 work in progress, not ready to merge * DUGK-61 The test works for now * DUGK-61 Fixed loop issue in menu test --------- Co-authored-by: Omar <[email protected]>

Added gitattributes for line endings Convert to react-tabs. Add unit tests. simplify type TabData Husky now only checks certain directories within client

* removed husky * DUGK-65 Removed husky from scripts * DUGK-65 Pre-push Fixed. Now comparing to origin/current-branch instead of default * DUGK-65 Added config script and updated readme * DUGK-65 Added hook script to prepare * Formatting * DUGK-65 CodeClimate issues fixed * DUGK-65 Removed husky from readme. Also changed echo to printf

Raised threshold of similar-code to 45 (default for TS) Fixed a lot of configuration errors.

Fixed additional codeclimate issues in client scripts.

If coverage is not met, the whole pipeline breaks instead of continuing. This temp fix will run the tests twice. Collect coverage during second.

This allows test coverage below threshold to be uploaded.

Implemented global mocks. Removed fullsize from iframe props. Fixed all failing tests. Added 'src/util/envUtil.ts' as getter for global env for easier testing. 'test/__mocks__/' now contains global mocks for components and page. Removed all redundant render() functions from tests. Replaced 'test/unitTest/config' dir with 'test/unitTest/jest.setup.ts' Removed iframe mock. Needs to exists for tests. Can be located by title.

- E2E menu test optimization - Pipeline errors in test.bash

Omarley7 · 2023-04-20T10:38:41Z

Here's the update @prasadtalasila

Scope of this PR

Priority number 1 was to reduce CodeClimte issues. To achieve that the following changes have been made:

React-tabs have been implemented instead of MUI tabs to reduce code and complexity. It uses styled-components paradigm
All unit tests are now using global mocks and test utility. Greatly improves the implementation of the testing framework. Described in detail here.
Bash scripts now declare PATH before exporting
Removed middle routes like "workflows" and "components", as they are not needed yet and only increase the complexity
E2E test of menu has been successfully been reduced. await in for..of and forEach is not possible with async functions. Reduce can solve all these issues.

Pipeline improvements

Added .gittattibute so WSL can run scripts on Windows machines with configuring line endings locally
.codeclimate.yml has been updated from V1 to V2. Runs much better locally. It uses .eslintrc from the client since it was the only way we could make it work. You mentioned this as well in Next top-level changes to the project #24. Any reason why it was ruled out?
We have removed husky from the project, as git-hooks provide the same functionality and take care of Husky workflow mistakes #34 It has also been added to the <root>/README.md

Client improvements

.eslintrc has been corrected according to eslintrc check #36
Readme has been updated to improve installation processes (could use some feedback on this from @KarstenMalle or @Artin13. Does it make sense when you read it?)
React-Iframe has been reinstated. It was removed early on because its value wasn't understood in the beginning. Now we realize the type-safety is much better with this package.
Added interface description to data files in route directories
On the Digital Twin route, all iframe tabs now go to URL/label: e.g. https://example.com/models
Instead of using environment variables directly in the code, getter functions have been implemented in util/envUtils.ts Greatly improves testability.

Not covered yet

Pipeline (client.yml) is not catching test errors Failing unittest are not picked up by CI pipeline #42

KarstenMalle · 2023-04-20T11:12:22Z

@Omarley7

Readme looks good to us, only thing is that we have to use sudo for the bash script/install.bash, to install correctly since we use a Ubuntu VM, but i'm guessing it's not required with the suggested setup of "vagrant virtual machine".

…7/DTaaS-Bachelor-new-GUI into feature/distributed-demo

codeclimate · 2023-04-21T10:22:51Z

Code Climate has analyzed commit 76c87f2 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Duplication	2

View more on Code Climate.

Omarley7 requested a review from prasadtalasila April 10, 2023 13:58

Omarley7 mentioned this pull request Apr 10, 2023

Husky workflow mistakes #34

Closed

prasadtalasila requested changes Apr 11, 2023

View reviewed changes

SyntaxXeror force-pushed the feature/distributed-demo branch from 8678627 to 7152af5 Compare April 19, 2023 11:35

codeclimate bot reviewed Apr 19, 2023

View reviewed changes

client/test/unitTests/__mocks__/page_mocks.tsx Show resolved Hide resolved

Omarley7 and others added 17 commits April 20, 2023 11:41

DUGK-57 Updated readme instructions. (#36)

329b9bf

* DUGK-57 Updated readme instructions. Ultra clear build instructions. * DUGK-57 removed 'test'

DUGK-61 Refactored /e2e/Menu.test.ts (#37)

a14d738

* DUGK-61 Refactored /e2e/Menu.test.ts * DUGK-61 work in progress, not ready to merge * DUGK-61 The test works for now * DUGK-61 Fixed loop issue in menu test --------- Co-authored-by: Omar <[email protected]>

Development (#40)

c959fb6

Added gitattributes for line endings Convert to react-tabs. Add unit tests. simplify type TabData Husky now only checks certain directories within client

revert changes to git hooks

de137b1

DUGK-64 changed es version to 2022 (#41)

e82522e

move TabData type to TabRender (#44)

a13b6a5

DUGK-68 Removed 'Components.tsx' and renamed '...Data.ts' (#47)

b6f3063

Small refactor of Workbench (#46)

d7b3ebf

DUGK-67 Removed 'Workflows.tsx' and renamed '...Data.ts' (#45)

78d21fa

DUGK-66 Updated CodeClimate config to V2 (#43)

ef3406e

Raised threshold of similar-code to 45 (default for TS) Fixed a lot of configuration errors.

Removed husky from enb

777d555

Fixed test script continuing on error.

868a03a

Fixed additional codeclimate issues in client scripts.

Tempoary fix on CI pipeline with unittest.

2382819

If coverage is not met, the whole pipeline breaks instead of continuing. This temp fix will run the tests twice. Collect coverage during second.

Added continue on error for tests cases.

4a1d996

This allows test coverage below threshold to be uploaded.

Removed contine on error from test step

072008f

Omarley7 force-pushed the feature/distributed-demo branch from b3c0222 to 072008f Compare April 20, 2023 09:41

Postponing to next PR

3467ab0

- E2E menu test optimization - Pipeline errors in test.bash

Omarley7 requested a review from prasadtalasila April 20, 2023 10:49

Omarley7 and others added 6 commits April 21, 2023 09:39

Re-refactored E2E menu test (#49)

f64336c

config.bash format

a8c1f54

fixed minor issues

f8ff069

Removed continue on error

4197c91

Merge branch 'feature/distributed-demo' of https://github.com/Omarley…

ab035d6

…7/DTaaS-Bachelor-new-GUI into feature/distributed-demo

Added yarn develop to readme

76c87f2

Omarley7 mentioned this pull request Apr 21, 2023

Rebased and fixed comments for PR#25 #39

Closed

prasadtalasila merged commit 23330ad into INTO-CPS-Association:feature/distributed-demo Apr 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WebApp update - Code quality #37

WebApp update - Code quality #37

Omarley7 commented Apr 10, 2023 •

edited

Loading

codecov-commenter commented Apr 10, 2023 •

edited

Loading

prasadtalasila left a comment

prasadtalasila Apr 11, 2023

Omarley7 Apr 20, 2023

prasadtalasila Apr 11, 2023

prasadtalasila Apr 11, 2023

prasadtalasila Apr 11, 2023

Omarley7 Apr 20, 2023

prasadtalasila commented Apr 11, 2023

prasadtalasila commented Apr 20, 2023

Omarley7 commented Apr 20, 2023

Omarley7 commented Apr 20, 2023 •

edited

Loading

KarstenMalle commented Apr 20, 2023 •

edited

Loading

codeclimate bot commented Apr 21, 2023

WebApp update - Code quality #37

WebApp update - Code quality #37

Conversation

Omarley7 commented Apr 10, 2023 • edited Loading

codecov-commenter commented Apr 10, 2023 • edited Loading

Codecov Report

prasadtalasila left a comment

Choose a reason for hiding this comment

prasadtalasila Apr 11, 2023

Choose a reason for hiding this comment

Omarley7 Apr 20, 2023

Choose a reason for hiding this comment

prasadtalasila Apr 11, 2023

Choose a reason for hiding this comment

prasadtalasila Apr 11, 2023

Choose a reason for hiding this comment

prasadtalasila Apr 11, 2023

Choose a reason for hiding this comment

Omarley7 Apr 20, 2023

Choose a reason for hiding this comment

prasadtalasila commented Apr 11, 2023

prasadtalasila commented Apr 20, 2023

Omarley7 commented Apr 20, 2023

Omarley7 commented Apr 20, 2023 • edited Loading

Scope of this PR

Pipeline improvements

Client improvements

Not covered yet

KarstenMalle commented Apr 20, 2023 • edited Loading

codeclimate bot commented Apr 21, 2023

Omarley7 commented Apr 10, 2023 •

edited

Loading

codecov-commenter commented Apr 10, 2023 •

edited

Loading

Omarley7 commented Apr 20, 2023 •

edited

Loading

KarstenMalle commented Apr 20, 2023 •

edited

Loading