-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fix bugs in NAS UI (deferred from previous releases) #2552
Conversation
ultmaster
commented
Jun 12, 2020
- Add tooltips for buttons
- Fix issue when an edge is shared by multiple mutables
- Auto-fit graph at initialization
- Add unit-tests
src/nasui/src/graphUtils.test.ts
Outdated
@@ -0,0 +1,26 @@ | |||
import { Graph } from './graphUtils'; |
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.
Suggest to move all unit test stuff into test
directory like nnimanager.
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.
Here is the reference.
Jest will look for test files with any of the following popular naming conventions:
- Files with .js suffix in
__tests__
folders. - Files with .test.js suffix.
- Files with .spec.js suffix.
The .test.js / .spec.js files (or the tests folders) can be located at any depth under the src top level folder.
We recommend to put the test files (or tests folders) next to the code they are testing so that relative imports appear shorter. For example, if App.test.js and App.js are in the same folder, the test only needs to import App from './App' instead of a long relative path. Collocation also helps find tests more quickly in larger projects.
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'll move them to __tests__
folder. I checked nni manager conversion (test
folder) and jest doesn't recognize them automatically.
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.
OK.
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.
btw, nnimanager is using mocha for test, what is the reason that nasui select jest instead of mocha?
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 used the default settings provided by react-scripts.