Robottelo: IDEA: Extend assertRaises to handle error messages internally

[rochacbruno]

Currenlty we can do the following to check proper error message is raised:
(thanks to @renzon which introduced this feature)

with self.assertRaises(CLIReturnCodeError) as raise_ctx:
    User.create(options)
self.assert_error_msg(
    raise_ctx,
    u'Could not create the user:'
)

I wonder if in future we could extend the assertRaises method in our base classes to accept a new argument in its *kwargs expected_msg and handle the proper error mesage is raised internally as it has already the context of the error message and we can then fail either for wrong raised error or wrong error message.

Than in our test case code would be just as:

with self.assertRaises(CLIReturnCodeError, expected_msg=u'Could not create the user:'):
    User.create(options)

[rplevka]

https://docs.python.org/3.5/library/unittest.html#unittest.TestCase.assertRaisesRegex

edit: for those of you who are worried that the link points to Python v3.5 documentation, this assert is supported in v2.7 too:
https://docs.python.org/2.7/library/unittest.html#unittest.TestCase.assertRaisesRegexp

[rochacbruno]

Nice @rplevka I did not knew about that feature, did you see this @renzon ? does it fit our needs?

[sghai]

Guys, If we are planning to implement this then may be we should hold on changes around other modules with current implementation ?

[svtkachenko]

Tried to use assertRaisesRegex during fix of incorrect usage of context manager with assertRaises. Looks really bad with real-world long messages :)
Something like

with self.assertRaises(Exception) as context:
    ...
self.assertEqual(context.exception.message, msg)

looks better.

[rochacbruno]

@svtkachenko can you explain further, with some example on why it looks bad? if not nice we can so overwrite the assertRaises

[svtkachenko]

@rochacbruno For example, https://github.com/SatelliteQE/robottelo/blob/master/tests/foreman/ui/test_setting.py#L701
Now it looks like this:

                    with self.assertRaises(UINoSuchElementError) as context:
                        edit_param(
                            session,
                            tab_locator=self.tab_locator,
                            param_name=param_name,
                        )
                    self.assertEqual(
                        context.exception.message,
                        'Could not find edit button to update selected param'
                    )

With assertRaisesRegex it looks like this:

                    with self.assertRaisesRegex(
                            UINoSuchElementError,
                            'Could not find edit button to update selected '
                            'param'
                    ):
                        edit_param(
                            session,
                            tab_locator=self.tab_locator,
                            param_name=param_name,
                        )

@rplevka suggests to use variable and it looks much better, however after reviews of my PRs I often can see that one-time variables is bad idea :)

                    msg =  'Could not find edit button to update selected param'
                    with self.assertRaisesRegex(UINoSuchElementError, msg):
                        edit_param(
                            session,
                            tab_locator=self.tab_locator,
                            param_name=param_name,
                        )

[svtkachenko]

@rochacbruno Personally, I'd vote for assertRaisesRegex and separate variable.

[rochacbruno]

I am ok with the separate variable when needed or using the pattern

with self.assertRaisesRegex(
    UINoSuchElementError,
    'Could not find edit button to update selected param'   
):
    edit_param(
        session,
        tab_locator=self.tab_locator,
        param_name=param_name,
    )

I just want to know from @renzon if the assertRaisesRegex fits the needs of CLITestCase because he implemented the self.assert_error_msg method and maybe there is a special case there.

[rplevka]

@svtkachenko - the one-time variable will be a temporary solution as we should end up using variables for messages anyway - after we implement support of internationalization, we will take the appropriate strings frrom constants.py where localized strings will be stored in a form of dictionary.

[rochacbruno]

@rplevka I think we can use Babel and store messages in proper .po files, then we get translations using gettext

[rplevka]

@rochacbruno +1 in this very second I found where foreman stores .po files :) so we can take it from first hand

[abalakh]

Have we dropped the support of python 2.6 for robottelo? If not, then assertRaisesRegex is not suitable for us - it was only introduced in 2.7.

[rplevka]

+1 to drop it :)

[renzon]

The issue I faced was that CLIReturnError didn't expose message on Exception attribute message: https://github.com/SatelliteQE/robottelo/blob/master/robottelo/cli/base.py#L14. I changed that. More than that in python 3 there is no message attribute, the is an args attribute. So the problem happened when I tried assert both CliError and CLIREturnError: On the second the error message is present on stderr, not on message. So probablt it's not to show up on str representation of Exception: https://github.com/SatelliteQE/robottelo/blob/master/robottelo/test.py#L182

[rochacbruno]

+1 to drop 2.6 support and fix CLI*Error messages in order to use assertRaisesRegex

[omaciel]

@SatelliteQE/quality-engineers let me make a decision on behalf of the project then :) Since we control the environment where robottelo is run, "I hereby decide that we shall no longer support Python 2.6".

Please proceed now :)

[JacobCallahan]

+1 on our way to python 3 only!

[rochacbruno]

Now I guess we can do regarding this issue:

• [ ] Include in docs that 2.6 is not supported
• [ ] Change CLI*Error to include stderr in its __repr__ message
• [ ] Replace self.assert_error_msg with self.AssertRaisesRegex
• [ ] Implement the gettext using .po files with translations to wrap the error messages

[JacobCallahan]

I'm just going to put this out there, the attrs library is fantastic for making classes more easily dealt with. Automatically takes care of a number of things including repr, slots, input validation, etc... oh and it comes with clojure-like instance cloning/modification.
https://pypi.python.org/pypi/attrs/16.0.0
Of course we would have to partially rewrite robottelo/nailgun!! hahaha

[sghai]

@rochacbruno : +1 to your plan. Now we need someone who can take this up.

[pgagne]

Include in docs that 2.6 is not supported

Does this also mean we should (eventually) switch from unittest2 to just unittest?.

I noticed in areas we were importing unittest2 and according to this unittest2 is a backport of unittest for python 2.4-2.6

[renzon]

The issue I faced was that CLIReturnError didn't expose message on Exception attribute message: https://github.com/SatelliteQE/robottelo/blob/master/robottelo/cli/base.py#L14. I changed that. More than that in python 3 there is no message attribute, the is an args attribute. So the problem happened when I tried assert both CliError and CLIREturnError: On the second the error message is present on stderr, not on message. So probablt it's not to show up on str representation of Exception: https://github.com/SatelliteQE/robottelo/blob/master/robottelo/test.py#L182

[renzon]

@pgagne @pondrejk: mentioning you guys because you were working on this.

[Ichimonji10]

@pgagne unittest2 also ports Python 3 features back to Python 2.7. The docs are wrong.

[rochacbruno]

@renzon I think your comment above is a duplicate of https://github.com/SatelliteQE/robottelo/issues/3749#issuecomment-243118074

I answered saying we can fix Cli*Error to include stderr on its __repr__

[rochacbruno]

Yeah @Ichimonji10 are right, assertRaisesRegex is available on unittest2 which works perfectly in Python 2.7

[sghai]

@rochacbruno : Can we close this out ? Anyway, we can refer the discussion on this issue later.

[rochacbruno]

@sghai yes that has been implemented, the translations is different issue.