[dev2][conf] Improvements #11575
[dev2][conf] Improvements #11575
Conversation
| "core.doesnotexist:never", | ||
| "core:doesnotexist" | ||
| ]) | ||
| def test_core_confs_not_allowed_via_cli(conan_api, argparse_args, conf_name): |
There was a problem hiding this comment.
This test reads complex, too many mocks, etc. If it is a unit test of profile loader, why unittesting the get_profiles_from_args which is a cli helper, instead of testing the profile loader (that can be directly called by the Python API ProfileAPI without calling that cli helper?
There was a problem hiding this comment.
Because I wanted to test the function that is used by several commands at the same time. If I test the ProfileLoader I could not ensure that the commands are getting that function so I thought it was better covered so. That was the reason.
Talking about mocks, I don't think a mock for argparse is too much but I can change the test and call only the ProfileLoader function if you think is good enough.
There was a problem hiding this comment.
I just added another unittest for ProfileLoader. If you want, I can get rid of this one.
There was a problem hiding this comment.
No need to remove it now it is done, but in general, the definition of "unittest" is testing the unit that is getting the code changes, in this case the "ProfileLoader". It is not about ensuring that certain commands behave some way, that is an "integration" test. The test that you added is much simpler (less mocking), and it guarantees that no matter what changes in the command layer, new commands, refactors, etc, profiles will keep raising if bad conf is defined. For example the ProfileAPI sub-api was not being covered by the first test, but now it is. Thanks!
| "core.doesnotexist:never", | ||
| "core:doesnotexist" | ||
| ]) | ||
| def test_core_confs_not_allowed_via_cli(conan_api, argparse_args, conf_name): |
There was a problem hiding this comment.
No need to remove it now it is done, but in general, the definition of "unittest" is testing the unit that is getting the code changes, in this case the "ProfileLoader". It is not about ensuring that certain commands behave some way, that is an "integration" test. The test that you added is much simpler (less mocking), and it guarantees that no matter what changes in the command layer, new commands, refactors, etc, profiles will keep raising if bad conf is defined. For example the ProfileAPI sub-api was not being covered by the first test, but now it is. Thanks!
Changelog: Bugfix: Fixed the wrong behavior about writing
-c core.xxx=valuevia CLI.Changelog: Feature: Raising an exception if the internal configuration is used and it does not exist in the configuration list. It only affects
tools*andcore*ones.Changelog: Feature: Brought
conan config listcommand back.Docs: https://github.com/conan-io/docs/pull/XXXX
Closes: #11346