Skip to content
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

ConsoleInteractionTest.py: test_ask_for_actions_and_apply defined twice #6056

Closed
akshatkarani opened this issue Aug 5, 2019 · 13 comments · Fixed by #6147 · May be fixed by #6109
Closed

ConsoleInteractionTest.py: test_ask_for_actions_and_apply defined twice #6056

akshatkarani opened this issue Aug 5, 2019 · 13 comments · Fixed by #6147 · May be fixed by #6109

Comments

@akshatkarani
Copy link
Member

Change the name of one of the function to something more appropriate.
difficulty/newcomer

@charfole
Copy link

charfole commented Aug 8, 2019

Will there be difficulty/newcomer labels with this issue?
I‘m a newcomer so that I look for the issues with this label to practice.

@akshatkarani
Copy link
Member Author

Most probably, but you should only work on this issue after getting assigned. Please read our Newcomers Guidelines and Commit Guidelines

@charfole
Copy link

charfole commented Aug 9, 2019

Copy that,THANKS!

@jayvdb
Copy link
Member

jayvdb commented Aug 21, 2019

@charfole , sorry for the delay - assigned now

@charfole
Copy link

@jayvdb ,It's OK. Thanks for your assigning.

@sameshl
Copy link

sameshl commented Sep 9, 2019

As there has not been any activity on this issue for more that 15 days, Can someone assign me this. I would love to work on it.

@RJ722
Copy link
Member

RJ722 commented Sep 9, 2019

seems like @charfole isn't working on this. assigning @sameshl

@RJ722 RJ722 assigned sameshl and unassigned charfole Sep 9, 2019
@sameshl
Copy link

sameshl commented Sep 16, 2019

@RJ722 I looked at

def test_ask_for_actions_and_apply(self):
failed_actions = set()
action = TestAction()
do_nothing_action = DoNothingAction()
args = [self.console_printer, Section(''),
[do_nothing_action.get_metadata(), action.get_metadata()],
{id(do_nothing_action): do_nothing_action, id(action): action},
failed_actions, Result('origin', 'message'), {}, {}, {}]
with simulate_console_inputs('a', 'param1', 'a', 'param2') as generator:
action.apply = unittest.mock.Mock(side_effect=AssertionError)
ask_for_action_and_apply(*args)
self.assertEqual(generator.last_input, 1)
self.assertIn('TestAction', failed_actions)
action.apply = lambda *args, **kwargs: {}
ask_for_action_and_apply(*args)
self.assertEqual(generator.last_input, 3)
self.assertNotIn('TestAction', failed_actions)
def test_ask_for_actions_and_apply(self):
failed_actions = set()
action = TestAction()
args = [self.console_printer, Section(''),
[action.get_metadata()], {id(action): action},
failed_actions, Result('origin', 'message'), {}, {}, {}]
with simulate_console_inputs('a', 'param1', 'a', 'param2') as generator:
action.apply = unittest.mock.Mock(side_effect=AssertionError)
ask_for_action_and_apply(*args)
self.assertEqual(generator.last_input, 1)
self.assertIn('TestAction', failed_actions)
action.apply = lambda *args, **kwargs: {}
ask_for_action_and_apply(*args)
self.assertEqual(generator.last_input, 3)
self.assertNotIn('TestAction', failed_actions)
but was not able to figure out a better way to rename them. Could you please help me out?

Thanks!

@RJ722
Copy link
Member

RJ722 commented Sep 16, 2019

It seems like the both the functions are mostly identical and that the first function is also additionally testing with DoNothingAction. This leads to a lot of code being duplicated - we would ideally want only one test in which both of these conditions could be tested.

One idea off the top of my head to do so would be to have a list of args and run the with statement within a loop for every arg in args, but it might take a little poking around to do so.

I'm turning this into a difficulty/low issue. @sameshl you can continue working on this issue and if you face any difficulties, please let us know and we'd be happy to help. :-)

@MaskedCarrot
Copy link

MaskedCarrot commented Oct 16, 2019

As there has not been any activity on this issue for about a month now can I work on it?

@Purvanshsingh
Copy link

I am a newcomer can you please assign me this issue?

Dilshaad21 added a commit to Dilshaad21/coala that referenced this issue Jan 10, 2020
Reduced redundancy by appending the arg list
of the second function to the first one and
run it in a loop through all the arguments.

Closes coala#6056
Dilshaad21 added a commit to Dilshaad21/coala that referenced this issue Jan 25, 2020
Reduced redundancy in test_ask_for_actions_and_apply
function by appending the arg listof the second
function to the first one and run it in a loop
through all the arguments.

Closes coala#6056
Dilshaad21 added a commit to Dilshaad21/coala that referenced this issue Jan 25, 2020
Reduced redundancy in test_ask_for_actions_and_apply
function by appending the arg list of the second
function to the first one and run it in a loop
through all the arguments.

Closes coala#6056
@tyajush
Copy link

tyajush commented Oct 22, 2020

Hi I would like to contribute to this issue, can someone please assign me. I am just getting started with open source contributions.

@sky3760000
Copy link

if the work is not completed, can u please assign me this issue?

pilgrim2308 pushed a commit to pilgrim2308/coala that referenced this issue Mar 14, 2021
Reduced redundancy in test_ask_for_actions_and_apply
function by appending the arg list of the second
function to the first one and run it in a loop
through all the arguments.

Closes coala#6056
abhishalya pushed a commit to pilgrim2308/coala that referenced this issue Mar 29, 2021
Reduced redundancy in test_ask_for_actions_and_apply
function by appending the arg list of the second
function to the first one and run it in a loop
through all the arguments.

Closes coala#6056
pilgrim2308 pushed a commit to pilgrim2308/coala that referenced this issue Mar 30, 2021
Reduced redundancy in test_ask_for_actions_and_apply
function by appending the arg list of the second
function to the first one and run it in a loop
through all the arguments.

Closes coala#6056
pilgrim2308 pushed a commit to pilgrim2308/coala that referenced this issue Mar 30, 2021
Reduced redundancy in test_ask_for_actions_and_apply
function by appending the arg list of the second
function to the first one and run it in a loop
through all the arguments.

Closes coala#6056

Co-authored by: Sanchit Gupta <sanchitgupta.072@gmail.com>
pilgrim2308 added a commit to pilgrim2308/coala that referenced this issue Mar 30, 2021
Reduced redundancy in test_ask_for_actions_and_apply
function by appending the arg list of the second
function to the first one and run it in a loop
through all the arguments.

Closes coala#6056

Co-authored-by: Sanchit Gupta <sanchitgupta.072@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment