Add dependencies to package output info when binary is not found (close #3316) - #3438
Conversation
|
Looks good, thanks. Please add a test for it. |
| requires = "dep1/1.0@lasote/testing" | ||
| """ | ||
|
|
||
| self.client.save({"conanfile.py": dep1_conanfile.format(version="1.0")}, clean_first=True) |
There was a problem hiding this comment.
Instead of parameterizing the template, name and version, remove them and do conan create . PkgName/version@user/channel
There was a problem hiding this comment.
Didn't realize that this was possible, lesson learned!
| from conans.paths import CONAN_MANIFEST | ||
|
|
||
|
|
||
| class InstallMissingDependencie(unittest.TestCase): |
|
|
||
| class InstallMissingDependencie(unittest.TestCase): | ||
|
|
||
| def setUp(self): |
There was a problem hiding this comment.
Don't extract it to the setUp(), unless it is clear common code shared by several tests in the fixture. Just put the preparation code in the test itself.
|
|
||
| self.client.save({"conanfile.py": foo_conanfile.format(dep1_version="2.0")}, | ||
| clean_first=True) | ||
| self.client.run("create . lasote/testing", ignore_error=True) |
There was a problem hiding this comment.
This is exactly the place you have to check the returned error from run()
There was a problem hiding this comment.
ok, I check for the return value. I thought that checking for the text itself in the following lines would be enough, but no, you are right, it is important to check that the command has failed
| class FooPkg(ConanFile): | ||
| name = "foo" | ||
| version = "1.0" | ||
| requires = "dep1/{dep1_version}@lasote/testing", "dep2/1.0@lasote/testing" |
There was a problem hiding this comment.
why 2 dependencies? The test can be perfectly done with 1.
There was a problem hiding this comment.
The issue we are testing is that dep2/1.0 (which depends on dep1/1.0) is not available when we bump dep1/1.0 to dep1/2.0 inside foo package.
There was a problem hiding this comment.
It could be tested such output, even if the binary is missing for another reason, so not really necessary to have such complex dependency graph. But I agree, it is nicer to test the full case, just in case.
There was a problem hiding this comment.
yes, you are right, we can remove dep1/1.0 from the repo and it should provide the same kind of information in the output message. It wouldn't fail for the same reason (override of transitive dependencies) but that behavior should have been tested at some other place.
I can leave here the minimum test and maybe create a new test module to store those tests that are 1-to-1 related to an specific issue.
There was a problem hiding this comment.
No need to do 2 tests, I think this one is ok here, just with some improvements.
| """ | ||
| self.client.save({"conanfile.py": foo_conanfile.format(dep1_version="1.0")}, | ||
| clean_first=True) | ||
| error = self.client.run("create . lasote/testing") |
|
|
||
| def missing_dep_test(self): | ||
| test_server = TestServer() | ||
| self.servers = {"myremote": test_server} |
There was a problem hiding this comment.
Finally, there is no upload, so you can remove the server.
| def missing_dep_test(self): | ||
| test_server = TestServer() | ||
| self.servers = {"myremote": test_server} | ||
| self.client = TestClient(servers=self.servers, users={"myremote": [("lasote", "mypass")]}) |
There was a problem hiding this comment.
Do not use self.client if client is possible
| # Create deps packages | ||
| dep1_conanfile = """from conans import ConanFile | ||
| class Dep1Pkg(ConanFile): | ||
| name = "dep1" |
There was a problem hiding this comment.
I know, but I prefer to keep the name in the recipe... at least to know but that conanfile.py is about. Is there any reason (internal to the machinery of Conan) to leave it out?
There was a problem hiding this comment.
Fair enough, you can leave the name.
| class FooPkg(ConanFile): | ||
| name = "foo" | ||
| version = "1.0" | ||
| requires = "dep1/{dep1_version}@lasote/testing", "dep2/1.0@lasote/testing" |
There was a problem hiding this comment.
No need to do 2 tests, I think this one is ok here, just with some improvements.
| # Create deps packages | ||
| dep1_conanfile = """from conans import ConanFile | ||
| class Dep1Pkg(ConanFile): | ||
| name = "dep1" |
There was a problem hiding this comment.
Fair enough, you can leave the name.
| import os | ||
| from conans.test.utils.cpp_test_files import cpp_hello_conan_files | ||
| from conans.util.files import load, save | ||
| from time import sleep |
There was a problem hiding this comment.
Unused imports. Try to enable a linter (like pep8, pycodestyle) in your IDE, it typically warns about unused imports (and unused variables, etc).
Also, try to follow the import order: first python ones, then dependencies, then conan imports.
Add dependencies to package output info when binary is not found (close conan-io#3316)
Current (1.7) ouput is:
we can easily improve the output listing the dependencies:
Changelog: Feature: Improve the output of a
conan installcommand printing dependencies when a binary is not found.