[PATCH 04/10] models: Add 'status' model

[Finucane, Stephen]

> On Wed, Sep 23, 2015 at 06:14:13PM +0100, Finucane, Stephen wrote:
> > As such, can you explain what advantages the above proposal would
> > introduce over the existing proposal? What does i915 need to do
> > differently? If not, I'd like to rework the patch per existing
> > comments (i.e. renaming 'Status' to 'Services' or 'Test Status') and
> > merge.
> 
> The amount of time needed to run i915 tests is so large that it's
> impractical to run them for each patch.
>
> Could you explain the use of the description and context fields? ('A
> label to discern status from statuses of other systems' really doesn't
> help much).

Sure.

* Description is a string returned by the CI. In the test proposal given by Thomas Monjalon, this could be the contents of the email sent by the CI system.
* Context is a non-unique name used to distinguish different tests (in the broader sense). Examples would include: intel/license-check, intel/unit-tests, dpdkorg/unit-tests/module1, dpdkorg/unit-tests/module2.

Is that clearer? I'm sorry but there is a limit of how much information I can include in the models before messing up the 'admin' display. I plan to add developer documentation in a follow up patch.

> What I'm assuming is that a lot of the "Status" rows will contain the
> same information, because it really belongs to a Test object. We're not
> just manipulating TestResults but Tests as well, so we need to describe
> those objects as well.
> 
> Another design question: How to you know with your model that all tests
> have been run? That's needed to determine that, indeed, that patch has
> passed all the defined tests (a "test" can of course hide a full test
> suite, summarized in one entry in patchwork).

Ah - I think you are misunderstanding what a "Status" represents here. A status does not represent the status of test that *has* to be run. It is a way of reporting the status of *any* tests that *have* run. Patchwork doesn't care what tests, if any, have been run: it is only an aggregator for results. This makes sense as patchwork is not the place to be storing configuration for or enforcing the tests that have to be run: all this management should be done using existing tools and mailing lists. This also allows smarts on the CI system whereby different tests can be run for different types of patches (a change to files in the 'docs' folder would run different tests than a change to 'src'). In summary, it is not necessary to define a distinction between tests and test results because we only care about the latter. Comprende? :)

> > One point regarding the "SeriesTestResult": I had planned to implement
> > this as a "lowest common denominator" property. That is, if a patch in
> > a series fail, the series is marked as fail etc. What you describe (a
> > unique, per series property) is quite a limited use case but its
> > behavior could be emulated by only testing the last patch in a series.
> > This would require very little "smarts" on your client end.
> 
> I don't think introducing special cases is a good thing, eg. how will
> you really differenciate the case of a test run on a full series and a
> test run on the last patch? you'll have to look at the other patches to
> see if a test run was run on them as well?

You're right. What you'd actually need to do is test the last patch and set the status on all patches. This might seem wasteful, but as far as you're concerned it's true (we have assumed that those patches work, so we must mark them as such). This has the benefit of allowing you to only test the last patch (and in theory, the series) for some time-consuming tests (i.e. integration tests) but keep doing per patch tests for other tests (i.e. license checks and unit tests). This "special case" won't exist in patchwork itself, only the client end (i.e. a custom script using the API. We won't be providing a client as it's too implementation specific).

As far as series support goes, I'll leave that element of implementation to you but I'd suggest looping through each SeriesPatch.status and accumulating the results for each context. If any patch if missing a context found in one or more other patches, you can set the series state to 'in progress'. Otherwise, use the LCD approach. How's that sound?

> Patchwork is used by projects with widly different needs, no special
> case like that in the code sounds better to me: testing a patch? results
> attached to a patch. testing a full series? results attached to a
> series.
> --
> Damien