Add cli for description #259
Conversation
|
Please tell me that there is any problem in |
tests/test_cli.py
Outdated
| def test_edit_description(runner, todos, todo_factory): | ||
| todo_factory(summary='harhar\n', description='Parnidi') | ||
|
|
||
| result = runner.invoke(cli, ['edit', '1', '--description', 'Parnidi']) |
There was a problem hiding this comment.
When editing the todo, set the description to something different than the original value. Otherwise it's unclear if the edit did anything or not.
tests/test_cli.py
Outdated
|
|
||
|
|
||
| def test_edit_description(runner, todos, todo_factory): | ||
| todo_factory(summary='harhar\n', description='Parnidi') |
| @@ -652,6 +653,9 @@ def todos(self, lists=(), priority=None, location='', category='', grep='', | |||
| if priority: | |||
| extra_where.append('AND PRIORITY > 0 AND PRIORITY <= ?') | |||
| params.append('{}'.format(priority)) | |||
| if description: | |||
There was a problem hiding this comment.
Oh, also, no tests seem to cover this new filter.
There was a problem hiding this comment.
Sorry, I think this filter does not do anything. so, I remove it.
| @@ -466,6 +468,8 @@ def move(ctx, list, ids): | |||
| @cli.command() | |||
| @pass_ctx | |||
| @click.argument('lists', nargs=-1, callback=_validate_lists_param) | |||
| @click.option('--description', | |||
There was a problem hiding this comment.
Since you deleted the implementation for this flag, the flag should be removed too.
Note that that implementation was not wrong; just missing tests though.
There was a problem hiding this comment.
I really don't understand which tests are missing.
And if I remove the flag then also it works correctly then why that implementation was not wrong.
There was a problem hiding this comment.
This flag was used here. By deleting that bit of code, this flag is now unused and ignored.
My initial comment on that related bit of code was that there were no tests for it (though the code itself seemed fine). Basically, that would be a test that uses runner with 'list', '--description'.
You can either remove the flag (now unused), or undelete that code and add tests for it (I'm very much prefer the latter, of course. 🙂 ).
|
can you please tell me where should I add these tests.( from test_model.py
or test_cli.py)
…On Tue, Jun 20, 2017 at 2:44 PM, Hugo Osvaldo Barrera < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In todoman/cli.py
<#259 (comment)>:
> @@ -466,6 +468,8 @@ def move(ctx, list, ids):
@cli.command()
@pass_ctx
@click.argument('lists', nargs=-1, callback=_validate_lists_param)
***@***.***('--description',
This flag was used here
<09fcd21#diff-d7b704f0d282081b24d6d106d9ea528fL656>.
By deleting that bit of code, this flag is now unused and ignored.
My initial comment on that related bit of code was that there were no
tests for it (though the code itself seemed fine). Basically, that would be
a test that uses runner with 'list', '--description'.
You can either remove the flag (now unused), or undelete that code and add
tests for it (I'm very much prefer the latter, of course. 🙂 ).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#259 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AXknkfySDlmU5I95-bV5tf0Pn-kKJeFwks5sGBMZgaJpZM4N9dDs>
.
--
Anubha Agrawal,
Sophomore year B.tech,
Computer Science Department
Zakir Hussain College of Engineering & Technology, AMU
|
tests/test_cli.py
Outdated
| assert 'Kimple' in result.output | ||
|
|
||
|
|
||
| def test_filter_description(runner, create): |
There was a problem hiding this comment.
Please also a second todo to this test case that is NOT included in the result (eg: one that has a descripcion that doesn't contain 'Kimple'). That way, we can make sure we're not showing todos that don't match the filtering criteria.
| ) | ||
| result = runner.invoke(cli, ['list']) | ||
|
|
||
| assert 'Shubik' in result.output |
There was a problem hiding this comment.
Also, assert that the other todo is not in result.output. You'll want to give them different summaries to tell them apart.
|
can you please tell me that is any other change is required?? |
|
Looks like this comment hasn't been addressed yet. |
Solve #240 for description