verdi config list: Do not except if no profiles are defined
#5921
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sphuber
force-pushed
the
fix/5920/verdi-config-list-no-profiles
branch
2 times, most recently
from
907a7f5 to
8f6eadd
Compare
|
With #5846 merged, this is now ready for review |
ltalirz
approved these changes
Mar 12, 2023
tests/conftest.py
Outdated
Comment on lines
518
to
519
| ``use_subprocess == False``. Normally this is done by the ``VerdiContext`` of the ``verdi`` command when | ||
| invoked normally from the command line, but when using the test runner in this interpreter, the object has |
There was a problem hiding this comment.
Optional
Suggested change
| ``use_subprocess == False``. Normally this is done by the ``VerdiContext`` of the ``verdi`` command when | |
| invoked normally from the command line, but when using the test runner in this interpreter, the object has | |
| ``use_subprocess == False``. When invoking ``verdi`` from the command line (and when running tests with ``use_subprocess == True``), | |
| this is done by the ``VerdiContext``, but when using the test runner in this interpreter, the object has |
The command should work if no profiles exist, in which case it should simply list the global values, however, it was excepting since it assumed that `ctx.obj.profile` would be set. This is set by the `ProfileParamType` which is used by the `--profile` option, but this is not called if no profiles are defined. To create a regression test, the `empty_config` fixture is used. However, since this fixture works in memory, the `use_subprocess=False` switch has to be used for `run_cli_command`. This by default would populate the `ctx.obj.profile` object. This is normally done by the `--profile` option, but this is not hit when using `click.CliRunner`. This would nullify the test, as it would always pass, even without the fix. To solve this issue, a new argument `initialize_ctx_obj` is added to the `run_cli_command` fixture. If set to `False`, the `ctx.obj` is not initialized. By using this new argument, the regression test properly failed before the test, but passes after the fix.
sphuber
force-pushed
the
fix/5920/verdi-config-list-no-profiles
branch
from
8f6eadd to
5bfc060
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #5920
The command should work if no profiles exist, in which case it should
simply list the global values, however, it was excepting since it
assumed that
ctx.obj.profilewould be set. This is set by theProfileParamTypewhich is used by the--profileoption, but this isnot called if no profiles are defined.
To create a regression test, the
empty_configfixture is used.However, since this fixture works in memory, the
use_subprocess=Falseswitch has to be used for
run_cli_command. This by default wouldpopulate the
ctx.obj.profileobject. This is normally done by the--profileoption, but this is not hit when usingclick.CliRunner.This would nullify the test, as it would always pass, even without the
fix.
To solve this issue, a new argument
initialize_ctx_objis added to therun_cli_commandfixture. If set toFalse, thectx.objis notinitialized. By using this new argument, the regression test properly
failed before the test, but passes after the fix.