Check if container components are started

[davidbrochart]

@agronholm This is what I was thinking about, to check if container components are started. I can add tests if you think this is fine.

[coveralls]

coverage: 90.834% (-0.6%) from 91.404%
when pulling 366a686 on davidbrochart:check-started
into a5822ac on asphalt-framework:5.0.

[agronholm]

It should find all the container components recursively, yes?

[davidbrochart]

Yes, the tests will check that.

[agronholm]

The one problem I (still) have with this is that such issues won't be caught in any tests, since only the runner will check for them.

[davidbrochart]

Sure, but don't you think that in tests we know better what we are doing, as developers, and that what we want to prevent is users forgetting to start child components? Also, I think that if we forget to start a container component in tests, they will very likely fail.

[agronholm]

I'm not quite grasping your logic here. What's the use case that would pass tests but fail at run time, necessitating this change?

[davidbrochart]

All I'm saying is that if we forget to start a container component, some resources won't be added and this is likely to have visible consequences in tests, which should probably fail.

[agronholm]

Sure, but if those tests fail, what do we need this extra check for in the runner?

[davidbrochart]

The runner is a user-facing API, and I think it is fine if it offers more features than running components with a lower-level API as it is done in tests. The problem with container components not started in a user application is that the app will not necessarily crash, but it will behave in a way that can be hard to debug. For instance, a file change watcher that doesn't notify file changes; it will need to identify where the issue comes from, etc. And users don't necessarily write exhaustive tests for their app, so it is nice to catch these errors ahead of time.
Actually check_started(root_component) can also be used in tests after starting the root component, if we want to. But I think what you would like is for this check to be done without any additional function call, directly by calling await root_component.start()?

[agronholm]

For instance, a file change watcher that doesn't notify file changes

This may be a bad example; I at least don't see how this relates to container components.

so it is nice to catch these errors ahead of time.

But that's just it – you'd have to explicitly check that await super().start() is called in order to catch these errors ahead of time.

for this check to be done without any additional function call, directly by calling await root_component.start()?

Again, how would that work? Didn't we already establish that that the superclass cannot raise any alarms unless the superclass method is called, at which point you wouldn't have to do it anyway?

I don't see any way out of this without a start_component() function which I had planned for v5.0 but had to be cut out due to the downsizing of the scope. That function would handle starting all of the child components, thus simplifying the component implementation.

[davidbrochart]

For instance, a file change watcher that doesn't notify file changes

This may be a bad example; I at least don't see how this relates to container components.

The service that watches file changes would be added by a child component that would not have been started by a container component.

so it is nice to catch these errors ahead of time.

But that's just it – you'd have to explicitly check that await super().start() is called in order to catch these errors ahead of time.

I mean with this PR this check is done automatically for you.

for this check to be done without any additional function call, directly by calling await root_component.start()?

Again, how would that work? Didn't we already establish that that the superclass cannot raise any alarms unless the superclass method is called, at which point you wouldn't have to do it anyway?

Yes it might not be possible, that's why I think this PR is at least an improvement, as it won't let you start the application if a container component forgets to await super().start(), at which point you can go and fix it.

[agronholm]

Obsoleted by #119.