-
Notifications
You must be signed in to change notification settings - Fork 3
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
RSDK-8003: Unit tests #6
Conversation
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.
Nice work! Not a lot of changes to do, just a few things I noticed
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.
Just some questions I have before approving
@@ -12,7 +12,7 @@ | |||
"build": { | |||
"build": "./build.sh", | |||
"path": "dist/archive.tar.gz", | |||
"arch": ["darwin/arm64", "darwin/amd64", "linux/arm64", "linux/amd64"] | |||
"arch": ["linux/arm64", "linux/amd64"] | |||
}, | |||
"entrypoint": "dist/main" |
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.
Is the entrypoint dist/__main__
or dist/main
? it seems like the the Makefile and the meta.json do different things, they should be consistent
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.
that's my bad, fixing this
src/model_inspector/input_tester.py
Outdated
from src.model_inspector.utils import is_defined_shape, output_to_shape_dict | ||
from model_inspector.utils import is_defined_shape, output_to_shape_dict |
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 it no longer necessary to use the src root directory?
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 moved the test_local.py into src because the imports were acting up when I ran make file
added unit tests, makefile, meta.json, and github workflows
viam build local works on Darwin
Also tested with client using viam server