Skip to content

Commit

Permalink
Added 'incorrect' tests for mode, source and dest.
Browse files Browse the repository at this point in the history
These arguments have a fixed sets of allowed values. The tests
are checking that the code correctly displays error information
when an unexpected argument value is supplied.
  • Loading branch information
Evildoor committed Dec 18, 2018
1 parent 42877e2 commit a5e428b
Showing 1 changed file with 36 additions and 0 deletions.
36 changes: 36 additions & 0 deletions Utils/Dataflow/pyDKB/dataflow/stage/tests/test_ProcessorStage.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,21 @@ def test_input_files(self):
self.args['input_files'] = ['something', 'something_else']
self.check_args()

def test_m_incorrect(self):
[msg, result] = isolate_function_error(self.stage.parse_args,
['-m', 'i'])
err = "error: argument -m/--mode: invalid choice: 'i' "\
"(choose from 'f', 's', 'm')"
self.assertIn(err, msg)

def test_mode_incorrect(self):
[msg, result] = isolate_function_error(self.stage.parse_args,
['--mode',
'incorrect'])
err = "error: argument -m/--mode: invalid choice: 'incorrect' "\
"(choose from 'f', 's', 'm')"

This comment has been minimized.

Copy link
@mgolosova

mgolosova Jan 21, 2019

Collaborator

Most possibly that the "correct" choices will still be one-character in the future, so the variant with 'incorrect' looks more promising than the one with 'i'.

Yet I have a question: are you sure that the part with '(choose from: ...)' should be specified explicitly? I mean, if one adds, I don`t know, mode 'k' -- this test still shouldn`t fail, right? Can we omit this part?

This comment has been minimized.

Copy link
@Evildoor

Evildoor Jan 22, 2019

Author Contributor

I believe that you are right and the '(choose from: ...)' part should be removed. Will do, and in the add_arg_incorrect() too.

This comment has been minimized.

Copy link
@Evildoor

Evildoor Jan 23, 2019

Author Contributor

Done via rebase and force-push: 9dc9aa0.

self.assertIn(err, msg)


def add_arg(arg, val, short=False):
def f(self):
Expand All @@ -170,6 +185,25 @@ def f(self):
setattr(ProcessorStageArgsTestCase, 'test_%s_%s' % (arg, val), f)


def add_arg_incorrect(arg, vals, short=False):

This comment has been minimized.

Copy link
@mgolosova

mgolosova Jan 21, 2019

Collaborator

Looks a bit weird: having method constructor, you still have test_{m|mode}_incorrect() added manually.
I see that it is because you already have the variable args_to_add, which does not contain mode -- but maybe the constructor still can be used for mode methods as well? Are they different anywhere?

This comment has been minimized.

Copy link
@Evildoor

Evildoor Jan 22, 2019

Author Contributor

Yes, the constructor can be used. Will be even simpler when change per comment above will be implemented, because vals argument will be redundant. Will do.

This comment has been minimized.

Copy link
@Evildoor

Evildoor Jan 23, 2019

Author Contributor

Done, same as above.

if short:
val = 'i'
args = ['-' + arg[0], val]
fname = 'test_%s_%s' % (arg[0], val)
else:
val = 'incorrect'
args = ['--' + arg, val]
fname = 'test_%s_%s' % (arg, val)

def f(self):
[msg, result] = isolate_function_error(self.stage.parse_args, args)
err = "error: argument -%s/--%s: invalid choice: '%s' "\
"(choose from %s)" % (arg[0], arg, val,
', '.join(["'%s'" % c for c in vals]))
self.assertIn(err, msg)
setattr(ProcessorStageArgsTestCase, fname, f)


def add_mode(val, short=False):
def f(self):
if short:
Expand Down Expand Up @@ -226,6 +260,8 @@ def f(self):
add_override_hdfs(a, v)
for m in modes:
add_override_mode(a, v, m)
add_arg_incorrect(a, args_to_add[a], True)
add_arg_incorrect(a, args_to_add[a])


for m in modes:
Expand Down

0 comments on commit a5e428b

Please sign in to comment.