[Merge] lp:~jamesodhunt/upstart/bug-1238078 into lp:upstart

Steve Langasek steve.langasek at canonical.com
Thu Oct 24 07:20:12 UTC 2013


Review: Needs Fixing

Hi James,

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.

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

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.

I haven't had a chance yet to review the code; will do that tomorrow and send any further comments.
-- 
https://code.launchpad.net/~jamesodhunt/upstart/bug-1238078/+merge/190723
Your team Upstart Reviewers is subscribed to branch lp:upstart.



More information about the upstart-devel mailing list