Code review comment for lp://staging/~vrruiz/ubuntu-autopilot-tests/core-app-introspection

Revision history for this message
Leo Arias (elopio) wrote :

Well, this can actually harm because it means that if one of those apps breaks during launch, this project will be blocked.
That's related to the basic things that we discussed for this project to succeed:
 - Nothing will land on this project if it's not already tested on isolation on the corresponding project.
 - We can't be blocked here by a problem on a single application. The only bugs that we will find with this project are the ones that involve two apps or more.

Don't get me wrong. This is a really nice idea, but I think it's missing the part about testing in isolation. I would like to see every one of those projects with something like:

application = calendar_app.CalendarApp.launch()

And with the a self test, of course:

def test_calendar_app_launch_must_return_introspection_proxy(self)

At that point, the test will run as part of the ci train and of the daily smoke, so having the extra check here will harm a little because we will be running the same test twice. But that's not too bad, we can live with that. What's bad is that we miss the purpose of this project if we start landing things here that are not improving the quality of the landing process of the individual projects in isolation.

Let me say it again, this is really nice. We are missing something so obvious like making sure that an app can be launched. But please, split this code and send it to the each project. Once it starts landing, we can discuss if we want to duplicate the test here.

Something slightly similar, that IMO will fit nicely into this project would be to launch all the preinstalled applications through the launcher and through the apps scope. We are close to having all the helpers needed to do it.

review: Needs Fixing

« Back to merge proposal