Code review comment for lp:~lderan/ubuntu-autopilot-tests/ubuntu-autopilot-tests

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Thanks for these tests! Glad to see you trying out autopilot. I'll leave a few quick comments.

You should change the author to be yourself in these tests :-)
In gmusicbrowser/, I'm confused why you have terminalapp classes, etc. Is this correct? Actually it seems all of your are this way. They can be blank files instead if you don't have a class to put in them :-)

In the gmusicbrowser test;

I take it you want the window to be the only one?
windows =
self.assertTrue(len(windows) == 1)

This is ok, but note that mainWindow.title will never change as it's already set (well, the app could change it, but I don't think that's what we're looking for here :-) ) I'm not exactly sure you need the eventually here, but if so you should ensure you get the window as part of the lambda function.
mainWindow = windows[0]
self.assertThat(lambda: mainWindow.title, Eventually(Contains('gmusicbrowser')))

Reviewing several apps at once is a bit hard; ideally split these up in the future :-) Overall this is a great first start. It seems you can't get introspection to work for these apps is that correct? If so, I would still like to see a bit more of the application tested if at all possible. I know using process manager will severely limit you, but it's worthwhile to see how creative you can be. Ping me on IRC @ balloons anytime for help or further info. Looking forward to seeing your tweaks.

review: Needs Fixing

« Back to merge proposal