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

[Damien Lespiau]

On Wed, Sep 23, 2015 at 08:04:26PM +0100, Finucane, Stephen wrote:
> * 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.

So it does mean my assumption was correct, eg. 'intel/license-check'
will be repeated for every patch. Hence abstracting a Test table we can
use for good.

> > 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.

'pending' suggests otherwise.

> 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.

Why not? I don't see anything that would prevent the admins from
defining the list of tests we are expecting results for a certain
project. We can do interesting things with that information, like making
sure we've run all the tests for a patch and only send the test results
back to the ml with an "ok/ko" mail + links.

This doesn't go against patchwork being just a test result aggregator,
quite the opposite, I do agree there FWIW.

> 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? :)

I don't think that a compelling argument. For instance, in that case, we
could have a more explicit "skip" or "non necessary" state for tests not
run and have more visibility on what the testing system actually does
while still tracking testing completion.

And si, comprendo, no soy estupido.

> > > 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.

A different special case, but still a special case. I can totally see an
improvement to run a test on the full series as the first step and only
do per-patch testing when the full series fails to narrow down the
faulty patch(es). Sure you can then "override" the test result
per-patch, but we're just having special case after special case. Not a
good start IMHO.

-- 
Damien