Merge lp:~kissiel/checkbox/fix-1593732-broken-continue into lp:checkbox

[Maciej Kisielewski]

This MR fixes 'continue' action on resume screen in checkbox-converged.

To test it :
start any session in C-C (preferably with 2 tests selected)
quit at any point (before the end)
restat C-C
hit continue
the session should be on _the next_ test.

Or run autopilot - make autopilot in checkbox/checkbox-touch

[Paul Larson (pwlars)]

I have a couple of questions/suggestions below.

Also, I got a traceback while trying to run the tests myself, which could just be the result of running on my desktop. For some reason, xvfb doesn't seem to work well for me here. Maybe just double check?

Traceback (most recent call last):
  File "/tmp/checkbox/fix-1593732-broken-continue/checkbox-touch/tests/autopilot/checkbox_touch/test_checkbox_touch.py", line 332, in test_remove_all_removes
    objectName='startTestButton', enabled=True)
  File "/usr/lib/python3/dist-packages/autopilot/introspection/dbus.py", line 295, in wait_select_single
    return self.select_single(type_name, **kwargs)
  File "/usr/lib/python3/dist-packages/autopilot/introspection/dbus.py", line 244, in select_single
    raise StateNotFoundError(type_name_str, **kwargs)
autopilot.exceptions.StateNotFoundError: Object not found with name '*' and properties {'enabled': True, 'objectName': 'startTestButton'}.

Tips on minimizing the occurrence of this failure are available here: https://developer.ubuntu.com/api/autopilot/python/1.5.0/faq-troubleshooting/

Ran 16 tests in 731.655s
FAILED (failures=1)

review: Needs Information

[Maciej Kisielewski (kissiel)]

the assignments in conditional are absolutely +1 (I feel dumb not doing that in a first place)

As for using ['result'] without checking - all py functions called from qml are wrapped with a 'view' decorator that has 'code' and 'result' fields. Code is similar to http codes (e.g. 200 - OK), result is an actual object returned by the function. So it's unrelated to the 'result' in the real object.

I <3 name collisions.

Sidenote: I plan to get rid of that @view. As we're not really benefiting from having it.

[Maciej Kisielewski (kissiel)]

> I have a couple of questions/suggestions below.
>
> Also, I got a traceback while trying to run the tests myself, which could just
> be the result of running on my desktop. For some reason, xvfb doesn't seem to
> work well for me here. Maybe just double check?
>
> Traceback (most recent call last):
> File "/tmp/checkbox/fix-1593732-broken-continue/checkbox-
> touch/tests/autopilot/checkbox_touch/test_checkbox_touch.py", line 332, in
> test_remove_all_removes
> objectName='startTestButton', enabled=True)
> File "/usr/lib/python3/dist-packages/autopilot/introspection/dbus.py", line
> 295, in wait_select_single
> return self.select_single(type_name, **kwargs)
> File "/usr/lib/python3/dist-packages/autopilot/introspection/dbus.py", line
> 244, in select_single
> raise StateNotFoundError(type_name_str, **kwargs)
> autopilot.exceptions.StateNotFoundError: Object not found with name '*' and
> properties {'enabled': True, 'objectName': 'startTestButton'}.
>
> Tips on minimizing the occurrence of this failure are available here:
> https://developer.ubuntu.com/api/autopilot/python/1.5.0/faq-troubleshooting/
>
> Ran 16 tests in 731.655s
> FAILED (failures=1)

I was unable to get a failure...
This might have been a case of very slow plainbox startup...

[Paul Larson (pwlars)]

Makes sense, and thanks for the change. It's a minor thing, but is now smaller and more readable I think. +1

review: Approve

=== modified file 'checkbox-touch/py/checkbox_touch.py'
--- checkbox-touch/py/checkbox_touch.py	2016-06-13 15:28:18 +0000
+++ checkbox-touch/py/checkbox_touch.py	2016-06-20 10:46:02 +0000
@@ -477,16 +477,15 @@
             'outcome': test['outcome'],
             'comments': test.get('comments', pod.UNSET),
             'execution_duration': time.time() - test['start_time'],
-            'return_code': test['result'].return_code
         }
-        try:
+        if 'result' in test:
             # if we're registering skipped test as an outcome of resuming
             # session, the result field of the test object will be missing
+            builder_kwargs['return_code'] = test['result'].return_code
             builder_kwargs['io_log_filename'] = test['result'].io_log_filename
             builder_kwargs['io_log'] = test['result'].io_log
-        except KeyError:
-            pass
-
+        else:
+            builder_kwargs['return_code'] = 0
         result = JobResultBuilder(**builder_kwargs).get_result()
         self.assistant.use_job_result(job_id, result)
         self.index += 1