-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fix Issue #670 : bake context argument should match the case #671
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
base: main
Are you sure you want to change the base?
Conversation
…ase or it will fallback to the first item of the list
|
@CodiumAI-Agent /review |
PR Analysis
PR Feedback
How to use
|
|
@CodiumAI-Agent /ask What is the 'bake' context argument? |
|
Question: What is the 'bake' context argument? Answer: In these tests, the context argument is a dictionary that contains the configuration for the project template to be created. The key 'command_line_interface' specifies the type of command line interface to be used in the project, such as 'Click' or 'Argparse'. The 'bake' function uses this context to generate a new project with the specified configuration. For example, in the line |
|
@CodiumAI-Agent /improve |
| context = {'command_line_interface': 'Click'} | ||
| result = cookies.bake(extra_context=context) |
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.
Suggestion: The 'command_line_interface' value in the context dictionary is case sensitive. It's better to use lower case to avoid potential issues with case sensitivity in the future.
| context = {'command_line_interface': 'Click'} | |
| result = cookies.bake(extra_context=context) | |
| context = {'command_line_interface': 'click'} | |
| result = cookies.bake(extra_context=context) |
| context = {'command_line_interface': 'Argparse'} | ||
| result = cookies.bake(extra_context=context) |
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.
Suggestion: The 'command_line_interface' value in the context dictionary is case sensitive. It's better to use lower case to avoid potential issues with case sensitivity in the future.
| context = {'command_line_interface': 'Argparse'} | |
| result = cookies.bake(extra_context=context) | |
| context = {'command_line_interface': 'argparse'} | |
| result = cookies.bake(extra_context=context) |
| context = {'command_line_interface': 'Click'} | ||
| result = cookies.bake(extra_context=context) |
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.
Suggestion: The 'command_line_interface' value in the context dictionary is case sensitive. It's better to use lower case to avoid potential issues with case sensitivity in the future.
| context = {'command_line_interface': 'Click'} | |
| result = cookies.bake(extra_context=context) | |
| context = {'command_line_interface': 'click'} | |
| result = cookies.bake(extra_context=context) |
| context = {'command_line_interface': 'Argparse'} | ||
| result = cookies.bake(extra_context=context) |
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.
Suggestion: The 'command_line_interface' value in the context dictionary is case sensitive. It's better to use lower case to avoid potential issues with case sensitivity in the future.
| context = {'command_line_interface': 'Argparse'} | |
| result = cookies.bake(extra_context=context) | |
| context = {'command_line_interface': 'argparse'} | |
| result = cookies.bake(extra_context=context) |
|
@CodiumAI-Agent /describe |
1 similar comment
|
@CodiumAI-Agent /describe |
|
@CodiumAI-Agent /ask What do you mean by handle the case sensitivity issue within the code itself? |
|
Question: What do you mean by handle the case sensitivity issue within the code itself? Answer: In the context of this PR, it seems the author has changed the case of the 'command_line_interface' value in the context dictionary from lowercase ('click', 'argparse') to capitalized ('Click', 'Argparse'). This change is likely a workaround to a case sensitivity issue, but it doesn't actually handle the case sensitivity within the code itself. A better solution would be to modify the code that processes the 'command_line_interface' value to be case-insensitive. For example, this could be achieved by converting the input to a consistent case (like lower case) before processing it: cli = context['command_line_interface'].lower()This way, it wouldn't matter if the input was 'Click', 'click', 'CLICK', etc. The code would handle it correctly. |
|
@CodiumAI-Agent /ask Where is "the code that processes the 'command_line_interface' value"? |
|
Question: Where is the code that processes the 'command_line_interface' value? Answer: |
|
@CodiumAI-Agent /ask Find the code that processes the 'command_line_interface' value in https://github.com/hackebrot/pytest-cookies/blob/main/src/pytest_cookies/plugin.py - possibly in the 'cookies.bake' method. |
|
Question: Find the code that processes the 'command_line_interface' value in https://github.com/hackebrot/pytest-cookies/blob/main/src/pytest_cookies/plugin.py - possibly in the 'cookies.bake' method. Answer: The 'command_line_interface' value is likely processed in the 'cookies.bake' method as you suggested. This method probably takes the 'command_line_interface' value from the context dictionary and uses it to determine which command line interface to use for the project. In the PR, the 'command_line_interface' value in the context dictionary is changed from lowercase ('click' or 'argparse') to title case ('Click' or 'Argparse'). This suggests that the 'cookies.bake' method or some other part of the code is case-sensitive and expects the 'command_line_interface' value to be in title case. To find the exact code that processes the 'command_line_interface' value, you would need to look at the implementation of the 'cookies.bake' method in the pytest-cookies plugin. Look for a line where the 'command_line_interface' key is used to access a value from a dictionary. The code might look something like this: cli = context['command_line_interface']Then, the 'cli' variable would be used to determine which command line interface to use. The code might look something like this: if cli == 'Click':
# Use the Click command line interface
elif cli == 'Argparse':
# Use the argparse command line interfacePlease check the actual code in the pytest-cookies plugin to find the exact implementation. |
To my understanding, this would solve the issue describe in #670. It fallback to the first item in the list if the case is not respected. It becomes a problem then if you try swapping argument order.
Also, this change likely causes the argparse test to fail, but this is maybe expected as you are testing non Click input with the Click RunnerCli