Code review comment for lp://staging/~jamesodhunt/upstart/bug-1238078

Hi Steve,

> What's the difference between the upstart-reload-signal.json test and
> the upstart-no-job-environ.json test? AFAICS, there isn't one. If the
> serialization format hasn't changed between 1.9 and 1.10, we shouldn't
> add a redundant test here, it's unnecessary and confusing.
Ack - gone.

>
> Also, perhaps it would be a good idea to name these json data tests
> after the upstream versions they correspond to, so that it's clearer?
> For instance, upstart-reload-signal.json could be called
> 'upstart-1.9.json', and upstart-with-job-environ.json could be called
> 'upstart-1.11.json'.
Agreed. We just need to remember to update upstart-1.11.json if new
serialisation data is added prior to 1.11 being released.

> I'm not sure if (or how) the separate session-init tests are needed.
> They contain different data (since they're obviously taken from a
> running session init's state rather than a system init), but there
> shouldn't be any differences in the *format*, should there? If there are
> no differences, then I think the tests are redundant and should be
> omitted. If there are differences, please call this out more clearly so
> that it's understood at a glance what we're testing for.
Right - there is no difference in the format of the data in the session
init .json files: the Session Init tests were an attempt to ensure
correct deserialisation of a "non-default" job environment table (which
can only be non-default when running as a Session Init). However, I
think you are right: these tests aren't really adding much since to
test that setup. Really, we need to be able to feed the .json files
to an actual Session Init process and we don't currently have that
ability.

> I haven't had a chance yet to review the code; will do that tomorrow and
> send any further comments.
Thanks!

Branch updated.