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

WIP: CLI testing #310

Closed
wants to merge 3 commits into from
Closed

WIP: CLI testing #310

wants to merge 3 commits into from

Conversation

mirielka
Copy link
Contributor

@mirielka mirielka commented Dec 6, 2016

Here is basic part of CLI testing for you to take a look at and provide feedback, before it's all done and polished up.

How it works: so far it's only tuned to run stageuser tests, so use ./make-tests ipatests/test_xmlrpc/test_stageuser_plugin.py --cli to run in CLI mode, or of course without the --cli option to run original API tests. Note that last three tests are expected to fail in CLI mode as group tracker has not been modified for CLI testing yet.

What gets changed:

  • '--cli' option is added
  • base tracker: changes that should ensure the basic functionality to run existing tests in CLI mode, this includes way how to execute commands and mapping of defined API options and output values to the CLI style (the idea of the mapping was to convert the text output from CLI to API style output so that more extensive methods to compare CLI output with expected values do not have to be created)
  • specific trackers: changes are needed because CLI calls have different output than API calls (some messages are different, some items of API output are not present in CLI). So far I've modified stageuser tracker fully (i.e. that one should be final) and user tracker partly (just so that stageuser tests that use user tracker work properly, but user tests will not work in CLI mode yet).
  • xmlrpc_test.py: unfortunately I failed to find a way how to raise the exception from subprocess so that it would fit the exception comparison using raises_exact method, so I chose to raise ExecutionError when it occurs in subprocess and compare just the error message in stderr. Since I expect the CLI tests to be run along with API tests, any change in type of raised error should be cought by API tests.

So far I observed:

  • positives: no tests need to be changed, all the necessary changes are in trackers (which was the intention)
  • negatives: only works for tracker based tests, i.e. not for declarative tests or tests that use just api.Command style; tests run longer that API tests

So please, if you have any comment and suggestions, I'll be glad to hear them so that I can polish this and finish the modifications.

Add --cli option to ipa-run-tests/make-test command
Extend base tracker to have functionality to run existing tests in CLI mode if --cli option is specified.
Modify specific (so far stageuser and user) trackers to properly handle CLI testing.
"""
cmd = "ipa {}".format(to_cli(name))
for item in args:
cmd += " {}".format(item)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean?

cmd = cmd + ' '.join(args)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, silly me, I forgot how to python. Will implement both your suggestions, thank you!

elif key in self.novalue and not options[key]:
continue
else:
if type(options[key]) is unicode and ' ' in options[key]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you cannot use is unicode it is not in Py3, so in Py3 is should be

if six.PY3:
    unicode = str

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or even better please use isinstance() instead of type()

elif value == 'False':
value = False
else:
value = unicode(value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unicode is not in PY3 so please apply the same steps as I mentioned above with six

continue
elif ':' in line:
key = self.mapping_output[line.split(':')[0].strip()]
value = line.split(':')[1].strip()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does this work with multiple values returned?


if key in modresult['result']:
modresult['result'][key].append(value)
elif value is True or value is False:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isinstance(value, bool) may be better

if error and not self.skip_error:
# if stderr is not empty and the error is not supposed to be
# ignored, raise error
raise(errors.ExecutionError(message=unicode(error[12:-1])))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does that hardcoded slice mean [12:-1]
You don't need ( ) after raise, it is not function

# ignored, raise error
raise(errors.ExecutionError(message=unicode(error[12:-1])))
elif self.skip_error:
self.skip_error = False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you change self.skip_error to False here?

try:
result = cmd(*args, **options)
except Exception as e:
print('Ran command: %s(%s): %s: %s' % (cmd, args_repr,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

print('Ran command: {cmd}(args): {t}: {e}'.format(
    cmd=cmd, args=args_repr, t=type(e).__name___, e=e
))

is preferred

@pvoborni
Copy link
Member

pvoborni commented Mar 8, 2017

Marking as postponed. We cannot expect the changes to be addressed by @mirielka any time soon. And CLI testing might need design discussion.

@tiran tiran added the WIP Work in progress - not ready yet for review label Dec 20, 2017
@pvoborni pvoborni removed the WIP Work in progress - not ready yet for review label Feb 6, 2018
@pvoborni
Copy link
Member

pvoborni commented Feb 7, 2018

This PR is pretty outdated and no development is happening. Therefore closing but also adding to: https://www.freeipa.org/page/Postponed

@pvoborni pvoborni closed this Feb 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants