[OPEN-ILS-GENERAL] Testing and Evergreen's quality (was: Database schema deprecation/supersedes stuff)

[Dan Scott]

Hi Mike:

On Wed, Jun 22, 2011 at 2:10 PM, Mike Rylander <mrylander at gmail.com> wrote:
> Top-posting because my response (or, more appropriately, follow-up) is
> in the form of a public google doc.
>
> Short version: We agree on all the goals, so I propose a way forward
> that leverages the size (bigger than pre-2.0 and growing) of our
> community and IMO gives us the tools to enact change toward those
> shared goals.
>
> Long version: https://docs.google.com/document/d/1EyLZ9PH25kwQvbC9uvYo0z8cYG5Ui3bFrzlkp2C2FYA/edit?hl=en_US

I haven't responded before now because I was hoping for others to join
in, but oh well.

I'm not sure we're trying to solve the same problems. In your Google
Doc, you state that "a new pain point has developed that will only get
worse if not addressed" and your CommitFest proposal hopes "to
encourage broader collaboration and better targeting of resources, and
to reduce the cost (both politically and in time) of context switching
between review/merge and active development of new features".

It's hard to respond succinctly because of the size of the proposal,
but some basic concerns I have are:
  * I don't see a direct linkage between the problems I raised about
our project's problems with quality releases and how the CommitFest
would solve those problems
  * If I'm reviewing one or two other patches while I'm trying to
respond to a reviewer's comments on my own patches during the 1 week
out of 8, I'm still going to be doing plenty of context-switching
during that week. And what happens during the other 7 weeks? Imposed
silence on IRC and the mailing lists so context doesn't get switched?
  * If I discover a new bug while I'm reviewing somebody else's code
during the CommitFest, I don't understand why I shouldn't open a new
bug at that time when I have the data and logs ready and available; it
seems likely to me that this will result in bugs that get forgotten
about and rediscovered later
  * I can't commit (hah) to being available full-time for CommitFest weeks
  * I don't understand the reference to a "political" cost of context-switching

My favourite part of the CommitFest document is the draft review
checklist. If we focused on ensuring the draft review checklist was
satisfied for every commit that hit the core repository - whether that
commit started life as a patch from a contributor, or was developed by
a committer - that would go a long way towards addressing some of the
quality problems that we have had in past release attempts. Maybe
rename the thing from "review checklist" to "commit checklist"?

If the core problem that the CommitFest proposal tries to solve is
making the limited amount of human power that we have more efficient,
why don't we invest some of that human power to take advantage of
machine power to provide us with feedback on quality and performance?
As an individual, when I review someone else's patch, I do the best I
can to set up a clean environment, load a representative set of data,
try different configuration switches, etc, but to do that properly
takes a massive amount of time, is repetitive and boring, and is still
subject to local variations in my environment. An automated testing
environment - or sets of environments - can do a far better job of
running repetitive / repeatable jobs than any one person can on their
own at testing and can objectively measure the performance of
different permutations (for example, "large academic" vs "large public
consortium" vs "special library" sets of data, configuration settings,
and workloads) and provide continuous feedback on how we're doing. The
different permutations help catch problems that we may forget about in
environments other than whatever our current focus may be.

On another front, I strongly suspect that nobody is regularly running
test migrations (with pre and post-migration workloads to test
functionality and performance regressions), but that's a relatively
easily repeatable task that I again strongly suspect is of a high
priority to most Evergreen sites. It would help flush out problems
like "authority code depends on every record having a 901c, but
migrated records may not have a 901c" that are hard to reproduce
without running data through a migration because freshly loaded data
all has the expected qualities...

Another way to alleviate a lack of human power is to add more
committers to the project. If there's a concern that new committers
might not hold code up to the same standards as the current committers
do, we should make those standards more clear (the draft review
checklist seems like a really good start at fleshing out that part of
the "Contributing" document) and ensure that we're holding up our own
code to those standards in the first place. Then we're in a much
better position to help contributors become committers who can then
help the project even more directly. If the commit checklist contains
"passes all unit tests" and "does not adversely affect performance of
system as measured by system test", then as we build up our unit tests
and a system test environment we have even less to fear from adding
committers to the project.