-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fix yaml2python convert_import for relative paths and prefixes #240
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.
the code looked good but unit tests don't really test imports with repositories because doesn't look up the repository's url
tests/test_dsl.py
Outdated
(dict(file="../foo.yaml"), "from ..foo import *"), | ||
(dict(file="../../foo.yaml"), "from ...foo import *"), | ||
(dict(repository="unfurl", file="foo.yaml"), "from unfurl.foo import *"), | ||
(dict(repository="repo", file="foo.yaml", namespace_prefix="tosca.ns", uri_prefix="tosca.ns"), "from repo import foo as tosca.ns"), |
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.
this should be tosca_repositories.repo
see this code in find_repository
return "tosca_repositories." + name, self.repository_paths[name]
i'm surprised the unit test didn't raise an error because of these lines in that function
tpl = self.template.tpl["repositories"][tosca_name or name]
url = normalize_path(tpl["url"])
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.
ah -- that only happens if the given repo is a URL; i'll add a test case for that. sort of tricky because the template is being mocked here.
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.
The repo is the name of repository not the url itself... I think the problem is the mock does have a repositories dictionary with the repo as a key
7957223
to
d4d04cc
Compare
this makes the type checker happy and enforces TOSCA spec
d4d04cc
to
b38b530
Compare
also clean up repo link function as a whole Co-Authored-By: Adam Souzis <[email protected]>
78aef0b
to
833f2ad
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Also adds some initial coverage tests for the changed function.
Draft for now, would appreciate another lookover to see if I missed anything.