Re: Code quality, principles and rules

[Edward Capriolo]

On Sat, Mar 18, 2017 at 9:21 PM, Qingcun Zhou  wrote:

> I wanted to contribute some unit test cases. However the unit test approach
> in Cassandra seems weird to me after looking into some examples. Not sure
> if anyone else has the same feeling.
>
> Usually, at least for all Java projects I have seen, people use mock
> (mockito, powermock) for dependencies. And then in a particular test case
> you verify the behavior using junit.assert* or mockito.verify. However we
> don't use mockito in Cassandra. Is there any reason for this? Without
> these, how easy do people think about adding unit test cases?
>
>
> Besides that, we have lots of singletons and there are already a handful of
> tickets to eliminate them. Maybe I missed something but I'm not seeing much
> progress. Is anyone actively working on this?
>
> Maybe a related problem. Some unit test cases have method annotated with
> @BeforeClass to do initialization work. However, it not only initializes
> direct dependencies, but also indirect ones, including loading
> cassandra.yaml and initializing indirect dependencies. This seems to me
> more like functional/integration test but not unit test style.
>
>
> On Fri, Mar 17, 2017 at 2:56 PM, Jeremy Hanna 
> wrote:
>
> > https://issues.apache.org/jira/browse/CASSANDRA-7837 may be some
> > interesting context regarding what's been worked on to get rid of
> > singletons and static initialization.
> >
> > > On Mar 17, 2017, at 4:47 PM, Jonathan Haddad 
> wrote:
> > >
> > > I'd like to think that if someone refactors existing code, making it
> more
> > > testable (with tests, of course) it should be acceptable on it's own
> > > merit.  In fact, in my opinion it sometimes makes more sense to do
> these
> > > types of refactorings for the sole purpose of improving stability and
> > > testability as opposed to mixing them with features.
> > >
> > > You referenced the issue I fixed in one of the early emails.  The fix
> > > itself was a couple lines of code.  Refactoring the codebase to make it
> > > testable would have been a huge effort.  I wish I had time to do it.  I
> > > created CASSANDRA-13007 as a follow up with the intent of working on
> > > compaction from a purely architectural standpoint.  I think this type
> of
> > > thing should be done throughout the codebase.
> > >
> > > Removing the singletons is a good first step, my vote is we just rip
> off
> > > the bandaid, do it, and move forward.
> > >
> > > On Fri, Mar 17, 2017 at 2:20 PM Edward Capriolo  >
> > > wrote:
> > >
> > >>> On Fri, Mar 17, 2017 at 2:31 PM, Jason Brown 
> > wrote:
> > >>>
> > >>> To François's point about code coverage for new code, I think this
> > makes
> > >> a
> > >>> lot of sense wrt large features (like the current work on
> > >> 8457/12229/9754).
> > >>> It's much simpler to (mentally, at least) isolate those changed
> > sections
> > >>> and it'll show up better in a code coverage report. With small
> patches,
> > >>> that might be harder to achieve - however, as the patch should come
> > with
> > >>> *some* tests (unless it's a truly trivial patch), it might just work
> > >> itself
> > >>> out.
> > >>>
> > >>> On Fri, Mar 17, 2017 at 11:19 AM, Jason Brown 
> > >>> wrote:
> > >>>
> >  As someone who spent a lot of time looking at the singletons topic
> in
> > >> the
> >  past, Blake brings a great perspective here. Figuring out and
> > >>> communicating
> >  how best to test with the system we have (and of course
> incrementally
> >  making that system easier to work with/test) seems like an
> achievable
> > >>> goal.
> > 
> >  On Fri, Mar 17, 2017 at 10:17 AM, Edward Capriolo <
> > >> edlinuxg...@gmail.com
> > 
> >  wrote:
> > 
> > > On Fri, Mar 17, 2017 at 12:33 PM, Blake Eggleston <
> > >> beggles...@apple.com
> > 
> > > wrote:
> > >
> > >> I think we’re getting a little ahead of ourselves talking about DI
> > >> frameworks. Before that even becomes something worth talking
> about,
> > >>> we’d
> > >> need to have made serious progress on un-spaghettifying Cassandra
> in
> > >>> the
> > >> first place. It’s an extremely tall order. Adding a DI framework
> > >> right
> > > now
> > >> would be like throwing gasoline on a raging tire fire.
> > >>
> > >> Removing singletons seems to come up every 6-12 months, and
> usually
> > >> abandoned once people figure out how difficult they are to remove
> > > properly.
> > >> I do think removing them *should* be a long term goal, but we
> really
> > > need
> > >> something more immediately actionable. Otherwise, nothing’s going
> to
> > >> happen, and we’ll be having this discussion again in a year or so
> > >> when
> > >> everyone’s angry that Cassandra 5.0 still isn’t ready for
> > >> production,
> > >>> a
> > >> year after it’s 

Re: [DISCUSS] Implementing code quality principles, and rules (was: Code quality, principles and rules)

[Ariel Weisberg]

 Brown <jasedbr...@gmail.com>
> wrote:
> 
> > re: code coverage (this could become it's own thread, and probably should)
> >
> > As somebody working on a new large feature (CASSANDRA-8457), I have a few
> > thoughts/observations about code coverage. As a preface, though, I'll point
> > out that we have jacoco (http://www.jacoco.org/jacoco/trunk/index.html)
> > already incorporated into the build, so it's quite easy to generate code
> > coverage reports ('ant codecoverage' runs the tests and creates the jacoco
> > binary, and 'ant jacoco-report' creates an html dump).
> >
> > As CASSANDRA-8457 is largely additive, it's pretty easy for me to confirm
> > the code coverage, as it's a whole new package - so clearly it's pretty
> > obvious to what i didn't cover in the test suite. Contrast this against the
> > rest of the code base. It may be harder to discern, from a simple
> > percentage output, just how much of an impact the added tests have across
> > the entire code base. Further, if a given patch added minor functionality
> > (say a one-liner fix), plus implementing some toString() functions, you
> > might actually see the coverage go *down* if the patch doesn't include
> > tests for the toString().
> >
> > Also, for things that just can't be unit tested in the current code base,
> > but where correctness can be ensured with a dtest, that wouldn't get
> > included by coverage tools. Of course, this is where incremental changes to
> > the code base to allow unit testing are really awesome and appreciated.
> > I've been encouraging some folks to do that with their submissions. It
> > makes their patches a bit more involved (for them to code and for me to
> > review), but hopefully it improves the overall quality of the code.
> >
> > Thus, imho, for new/large features, sure, having a high coverage is an
> > awesome thing: I'd think a minimum of 80%, with a "highly recommended"
> > target of > 90%. For smaller fixes, I'm not so sure, but I think a
> > reasonable agreement between the contributor and reviewer, based on the
> > guidelines coming from Blake's forthcoming doc, seem like a good point of
> > departure.
> >
> > On Tue, Mar 28, 2017 at 5:57 AM, Josh McKenzie <jmcken...@apache.org>
> > wrote:
> >
> > > Hold on now - there's no reason to throw the baby out with the bathwater
> > on
> > > this. Code coverage on it's own is admittedly a questionable metric, but
> > > bad metrics are better than *no* metrics if taken with context.
> > >
> > > Most code coverage tools will allow you to drill down to the granularity
> > > we'd need to confirm that new code (new, as in entirely new file, etc)
> > was
> > > covered, and to delta against a previously run coverage metric to make
> > sure
> > > there's an uptick in modules where we make changes. It's quite a bit more
> > > involved / extra overhead compared to what we do now; I'd argue we
> > probably
> > > need to learn to crawl before we can walk (as in, actually write tests
> > for
> > > the code that goes into the project and do a *lot* more jmh benchmarking
> > > for perf changes).
> > >
> > > Definitely like the direction this is headed, but it sounds like some
> > > cart-before-horse to me if we integrate point 4 with the rest of them.
> > Also
> > > - for point 2 who's going to do the work to correlate flaky failures to
> > the
> > > changes that introduced them, or are we just going to accept that flaky
> > > tests get added and deal with them retroactively? I assume we could also
> > > set up some kind of CI job that would run a new test, say, 20 times and
> > > greenlight it for patches adding completely new tests? Might be a bad
> > idea,
> > > might not.
> > >
> > > On Tue, Mar 28, 2017 at 1:13 AM, Blake Eggleston <beggles...@apple.com>
> > > wrote:
> > >
> > > > In addition to it’s test coverage problem, the project has a general
> > > > testability problem, and I think it would be more effective to
> > introduce
> > > > some testing guidelines and standards that drive incremental
> > improvement
> > > of
> > > > both, instead of requiring an arbitrary code coverage metric be hit,
> > > which
> > > > doesn’t tell the whole story anyway.
> > > >
> > > > It’s not ready yet, but I’ve been putting together a testing standards
> > > > document for the project since bringing it up in the “Code quality,
>

Re: [DISCUSS] Implementing code quality principles, and rules (was: Code quality, principles and rules)

[sankalp kohli]

If the code coverage goes down or do not go above the required cutoff due
to adding toString or getter setter like functionality, can we make it a
process to explain the reason in the JIRA before committing it?

Regarding changes in hard to unit tests parts of the code, can we make it a
process to link the JIRA which talks about rewriting that component of C*.
Linking JIRAs will help us learn how many changes have gone in with little
or no testing due to this component.

On Tue, Mar 28, 2017 at 8:38 AM, Jason Brown <jasedbr...@gmail.com> wrote:

> re: code coverage (this could become it's own thread, and probably should)
>
> As somebody working on a new large feature (CASSANDRA-8457), I have a few
> thoughts/observations about code coverage. As a preface, though, I'll point
> out that we have jacoco (http://www.jacoco.org/jacoco/trunk/index.html)
> already incorporated into the build, so it's quite easy to generate code
> coverage reports ('ant codecoverage' runs the tests and creates the jacoco
> binary, and 'ant jacoco-report' creates an html dump).
>
> As CASSANDRA-8457 is largely additive, it's pretty easy for me to confirm
> the code coverage, as it's a whole new package - so clearly it's pretty
> obvious to what i didn't cover in the test suite. Contrast this against the
> rest of the code base. It may be harder to discern, from a simple
> percentage output, just how much of an impact the added tests have across
> the entire code base. Further, if a given patch added minor functionality
> (say a one-liner fix), plus implementing some toString() functions, you
> might actually see the coverage go *down* if the patch doesn't include
> tests for the toString().
>
> Also, for things that just can't be unit tested in the current code base,
> but where correctness can be ensured with a dtest, that wouldn't get
> included by coverage tools. Of course, this is where incremental changes to
> the code base to allow unit testing are really awesome and appreciated.
> I've been encouraging some folks to do that with their submissions. It
> makes their patches a bit more involved (for them to code and for me to
> review), but hopefully it improves the overall quality of the code.
>
> Thus, imho, for new/large features, sure, having a high coverage is an
> awesome thing: I'd think a minimum of 80%, with a "highly recommended"
> target of > 90%. For smaller fixes, I'm not so sure, but I think a
> reasonable agreement between the contributor and reviewer, based on the
> guidelines coming from Blake's forthcoming doc, seem like a good point of
> departure.
>
> On Tue, Mar 28, 2017 at 5:57 AM, Josh McKenzie <jmcken...@apache.org>
> wrote:
>
> > Hold on now - there's no reason to throw the baby out with the bathwater
> on
> > this. Code coverage on it's own is admittedly a questionable metric, but
> > bad metrics are better than *no* metrics if taken with context.
> >
> > Most code coverage tools will allow you to drill down to the granularity
> > we'd need to confirm that new code (new, as in entirely new file, etc)
> was
> > covered, and to delta against a previously run coverage metric to make
> sure
> > there's an uptick in modules where we make changes. It's quite a bit more
> > involved / extra overhead compared to what we do now; I'd argue we
> probably
> > need to learn to crawl before we can walk (as in, actually write tests
> for
> > the code that goes into the project and do a *lot* more jmh benchmarking
> > for perf changes).
> >
> > Definitely like the direction this is headed, but it sounds like some
> > cart-before-horse to me if we integrate point 4 with the rest of them.
> Also
> > - for point 2 who's going to do the work to correlate flaky failures to
> the
> > changes that introduced them, or are we just going to accept that flaky
> > tests get added and deal with them retroactively? I assume we could also
> > set up some kind of CI job that would run a new test, say, 20 times and
> > greenlight it for patches adding completely new tests? Might be a bad
> idea,
> > might not.
> >
> > On Tue, Mar 28, 2017 at 1:13 AM, Blake Eggleston <beggles...@apple.com>
> > wrote:
> >
> > > In addition to it’s test coverage problem, the project has a general
> > > testability problem, and I think it would be more effective to
> introduce
> > > some testing guidelines and standards that drive incremental
> improvement
> > of
> > > both, instead of requiring an arbitrary code coverage metric be hit,
> > which
> > > doesn’t tell the whole story anyway.
> > >
> > > It’s not ready yet, but I’ve been putting together a testing stand

Re: [DISCUSS] Implementing code quality principles, and rules (was: Code quality, principles and rules)

[Jason Brown]

re: code coverage (this could become it's own thread, and probably should)

As somebody working on a new large feature (CASSANDRA-8457), I have a few
thoughts/observations about code coverage. As a preface, though, I'll point
out that we have jacoco (http://www.jacoco.org/jacoco/trunk/index.html)
already incorporated into the build, so it's quite easy to generate code
coverage reports ('ant codecoverage' runs the tests and creates the jacoco
binary, and 'ant jacoco-report' creates an html dump).

As CASSANDRA-8457 is largely additive, it's pretty easy for me to confirm
the code coverage, as it's a whole new package - so clearly it's pretty
obvious to what i didn't cover in the test suite. Contrast this against the
rest of the code base. It may be harder to discern, from a simple
percentage output, just how much of an impact the added tests have across
the entire code base. Further, if a given patch added minor functionality
(say a one-liner fix), plus implementing some toString() functions, you
might actually see the coverage go *down* if the patch doesn't include
tests for the toString().

Also, for things that just can't be unit tested in the current code base,
but where correctness can be ensured with a dtest, that wouldn't get
included by coverage tools. Of course, this is where incremental changes to
the code base to allow unit testing are really awesome and appreciated.
I've been encouraging some folks to do that with their submissions. It
makes their patches a bit more involved (for them to code and for me to
review), but hopefully it improves the overall quality of the code.

Thus, imho, for new/large features, sure, having a high coverage is an
awesome thing: I'd think a minimum of 80%, with a "highly recommended"
target of > 90%. For smaller fixes, I'm not so sure, but I think a
reasonable agreement between the contributor and reviewer, based on the
guidelines coming from Blake's forthcoming doc, seem like a good point of
departure.

On Tue, Mar 28, 2017 at 5:57 AM, Josh McKenzie <jmcken...@apache.org> wrote:

> Hold on now - there's no reason to throw the baby out with the bathwater on
> this. Code coverage on it's own is admittedly a questionable metric, but
> bad metrics are better than *no* metrics if taken with context.
>
> Most code coverage tools will allow you to drill down to the granularity
> we'd need to confirm that new code (new, as in entirely new file, etc) was
> covered, and to delta against a previously run coverage metric to make sure
> there's an uptick in modules where we make changes. It's quite a bit more
> involved / extra overhead compared to what we do now; I'd argue we probably
> need to learn to crawl before we can walk (as in, actually write tests for
> the code that goes into the project and do a *lot* more jmh benchmarking
> for perf changes).
>
> Definitely like the direction this is headed, but it sounds like some
> cart-before-horse to me if we integrate point 4 with the rest of them. Also
> - for point 2 who's going to do the work to correlate flaky failures to the
> changes that introduced them, or are we just going to accept that flaky
> tests get added and deal with them retroactively? I assume we could also
> set up some kind of CI job that would run a new test, say, 20 times and
> greenlight it for patches adding completely new tests? Might be a bad idea,
> might not.
>
> On Tue, Mar 28, 2017 at 1:13 AM, Blake Eggleston <beggles...@apple.com>
> wrote:
>
> > In addition to it’s test coverage problem, the project has a general
> > testability problem, and I think it would be more effective to introduce
> > some testing guidelines and standards that drive incremental improvement
> of
> > both, instead of requiring an arbitrary code coverage metric be hit,
> which
> > doesn’t tell the whole story anyway.
> >
> > It’s not ready yet, but I’ve been putting together a testing standards
> > document for the project since bringing it up in the “Code quality,
> > principles and rules” email thread a week or so ago.
> >
> > On March 27, 2017 at 4:51:31 PM, Edward Capriolo (edlinuxg...@gmail.com)
> > wrote:
> > On Mon, Mar 27, 2017 at 7:03 PM, Josh McKenzie <jmcken...@apache.org>
> > wrote:
> >
> > > How do we plan on verifying #4? Also, root-cause to tie back new code
> > that
> > > introduces flaky tests (i.e. passes on commit, fails 5% of the time
> > > thereafter) is a non-trivial pursuit (thinking #2 here), and a pretty
> > > common problem in this environment.
> > >
> > > On Mon, Mar 27, 2017 at 6:51 PM, Nate McCall <zznat...@gmail.com>
> wrote:
> > >
> > > > I don't want to lose track of the original idea from François, so
> > > > let's do this form

Re: [DISCUSS] Implementing code quality principles, and rules (was: Code quality, principles and rules)

[Josh McKenzie]

How do we plan on verifying #4? Also, root-cause to tie back new code that
introduces flaky tests (i.e. passes on commit, fails 5% of the time
thereafter) is a non-trivial pursuit (thinking #2 here), and a pretty
common problem in this environment.

On Mon, Mar 27, 2017 at 6:51 PM, Nate McCall  wrote:

> I don't want to lose track of the original idea from François, so
> let's do this formally in preparation for a vote. Having this all in
> place will make transition to new testing infrastructure more
> goal-oriented and keep us more focused moving forward.
>
> Does anybody have specific feedback/discussion points on the following
> (awesome, IMO) proposal:
>
> Principles:
>
> 1. Tests always pass. This is the starting point. If we don't care
> about test failures, then we should stop writing tests. A recurring
> failing test carries no signal and is better deleted.
> 2. The code is tested.
>
> Assuming we can align on these principles, here is a proposal for
> their implementation.
>
> Rules:
>
> 1. Each new release passes all tests (no flakinesss).
> 2. If a patch has a failing test (test touching the same code path),
> the code or test should be fixed prior to being accepted.
> 3. Bugs fixes should have one test that fails prior to the fix and
> passes after fix.
> 4. New code should have at least 90% test coverage.
>

[DISCUSS] Implementing code quality principles, and rules (was: Code quality, principles and rules)

[Nate McCall]

I don't want to lose track of the original idea from François, so
let's do this formally in preparation for a vote. Having this all in
place will make transition to new testing infrastructure more
goal-oriented and keep us more focused moving forward.

Does anybody have specific feedback/discussion points on the following
(awesome, IMO) proposal:

Principles:

1. Tests always pass. This is the starting point. If we don't care
about test failures, then we should stop writing tests. A recurring
failing test carries no signal and is better deleted.
2. The code is tested.

Assuming we can align on these principles, here is a proposal for
their implementation.

Rules:

1. Each new release passes all tests (no flakinesss).
2. If a patch has a failing test (test touching the same code path),
the code or test should be fixed prior to being accepted.
3. Bugs fixes should have one test that fails prior to the fix and
passes after fix.
4. New code should have at least 90% test coverage.

Re: Code quality, principles and rules

[Michael Shuler]

On 03/22/2017 12:41 PM, François Deliège wrote:
> A first actionable step is to increase the visibility of the test
> coverage.  Ideally this would be integrated in the Jenkins run on
> Apache.  Michael Shuler, is this something you can take a look at?
> Let me know if we can help.

We've been running the jacoco coverage ant target on cassci for a very
long time, but cassci will be going away in the future. I will not have
time to add coverage to ASF Jenkins in the foreseeable future, so help
would certainly be appreciated in all things testing for the Apache
Cassandra project.

Someone interested in including coverage runs could add to the Job DSL
groovy under the jenkins-dsl/ dir - this is where all the jobs are
configured on the ASF Jenkins instance. Dikang /msg'ed me on IRC earlier
about coverage, too.

Here's the cassandra-builds repo that drives everything on the project's
ASF Jenkins jobs:
https://git1-us-west.apache.org/repos/asf?p=cassandra-builds.git

Here's where all the current project jobs are on ASF Jenkins:
https://builds.apache.org/view/A-D/view/Cassandra/

Here's info about how the Job DSL works - all job configs are managed
with DSL, with the exception of a couple scratch utility jobs:
https://github.com/jenkinsci/job-dsl-plugin/wiki

Here's info about ASF Jenkins:
https://cwiki.apache.org/confluence/display/INFRA/Jenkins

Basically, my task over the last few months has been to migrate Apache
Cassandra testing jobs to run on the Apache Foundation's Jenkins and
sunset CassCI. 95% of that task is done.

-- 
Warm regards,
Michael

Re: Code quality, principles and rules

[François Deliège]

Thanks everybody for chiming in.  I have not heard any concerns about the 
rules, so I’d like to move forward with some concrete steps in that direction.

A first actionable step is to increase the visibility of the test coverage.  
Ideally this would be integrated in the Jenkins run on Apache.  Michael Shuler, 
is this something you can take a look at?  Let me know if we can help.

A second step, imho, is to get agreement among committers that no patch that 
decrease test coverage will be accepted by any committer.   Could a PMC throw 
this question as a vote?

Finally, I used my mad data analytics skills to look at the ratio of non 
committers contributors during the last 6 months.Turns out that 60% of the 
authors are not committers.   So as mentioned in the testing thread Jason 
started, when we think about testing, I think it makes sense to consider 
opening it up to non committers.   Today the testing time ranges from 2 hours 
for unitests to 1 day for integration tests.  I’d like to explore if we can 
throw some hardware at the problem.  I’d appreciate a pointer to who I should 
talk to.

Re: Code quality, principles and rules

[Edward Capriolo]

On Saturday, March 18, 2017, Qingcun Zhou  wrote:

> I wanted to contribute some unit test cases. However the unit test approach
> in Cassandra seems weird to me after looking into some examples. Not sure
> if anyone else has the same feeling.
>
> Usually, at least for all Java projects I have seen, people use mock
> (mockito, powermock) for dependencies. And then in a particular test case
> you verify the behavior using junit.assert* or mockito.verify. However we
> don't use mockito in Cassandra. Is there any reason for this? Without
> these, how easy do people think about adding unit test cases?
>
>
> Besides that, we have lots of singletons and there are already a handful of
> tickets to eliminate them. Maybe I missed something but I'm not seeing much
> progress. Is anyone actively working on this?
>
> Maybe a related problem. Some unit test cases have method annotated with
> @BeforeClass to do initialization work. However, it not only initializes
> direct dependencies, but also indirect ones, including loading
> cassandra.yaml and initializing indirect dependencies. This seems to me
> more like functional/integration test but not unit test style.
>
>
> On Fri, Mar 17, 2017 at 2:56 PM, Jeremy Hanna  >
> wrote:
>
> > https://issues.apache.org/jira/browse/CASSANDRA-7837 may be some
> > interesting context regarding what's been worked on to get rid of
> > singletons and static initialization.
> >
> > > On Mar 17, 2017, at 4:47 PM, Jonathan Haddad  > wrote:
> > >
> > > I'd like to think that if someone refactors existing code, making it
> more
> > > testable (with tests, of course) it should be acceptable on it's own
> > > merit.  In fact, in my opinion it sometimes makes more sense to do
> these
> > > types of refactorings for the sole purpose of improving stability and
> > > testability as opposed to mixing them with features.
> > >
> > > You referenced the issue I fixed in one of the early emails.  The fix
> > > itself was a couple lines of code.  Refactoring the codebase to make it
> > > testable would have been a huge effort.  I wish I had time to do it.  I
> > > created CASSANDRA-13007 as a follow up with the intent of working on
> > > compaction from a purely architectural standpoint.  I think this type
> of
> > > thing should be done throughout the codebase.
> > >
> > > Removing the singletons is a good first step, my vote is we just rip
> off
> > > the bandaid, do it, and move forward.
> > >
> > > On Fri, Mar 17, 2017 at 2:20 PM Edward Capriolo  >
> > > wrote:
> > >
> > >>> On Fri, Mar 17, 2017 at 2:31 PM, Jason Brown  >
> > wrote:
> > >>>
> > >>> To François's point about code coverage for new code, I think this
> > makes
> > >> a
> > >>> lot of sense wrt large features (like the current work on
> > >> 8457/12229/9754).
> > >>> It's much simpler to (mentally, at least) isolate those changed
> > sections
> > >>> and it'll show up better in a code coverage report. With small
> patches,
> > >>> that might be harder to achieve - however, as the patch should come
> > with
> > >>> *some* tests (unless it's a truly trivial patch), it might just work
> > >> itself
> > >>> out.
> > >>>
> > >>> On Fri, Mar 17, 2017 at 11:19 AM, Jason Brown  >
> > >>> wrote:
> > >>>
> >  As someone who spent a lot of time looking at the singletons topic
> in
> > >> the
> >  past, Blake brings a great perspective here. Figuring out and
> > >>> communicating
> >  how best to test with the system we have (and of course
> incrementally
> >  making that system easier to work with/test) seems like an
> achievable
> > >>> goal.
> > 
> >  On Fri, Mar 17, 2017 at 10:17 AM, Edward Capriolo <
> > >> edlinuxg...@gmail.com 
> > 
> >  wrote:
> > 
> > > On Fri, Mar 17, 2017 at 12:33 PM, Blake Eggleston <
> > >> beggles...@apple.com 
> > 
> > > wrote:
> > >
> > >> I think we’re getting a little ahead of ourselves talking about DI
> > >> frameworks. Before that even becomes something worth talking
> about,
> > >>> we’d
> > >> need to have made serious progress on un-spaghettifying Cassandra
> in
> > >>> the
> > >> first place. It’s an extremely tall order. Adding a DI framework
> > >> right
> > > now
> > >> would be like throwing gasoline on a raging tire fire.
> > >>
> > >> Removing singletons seems to come up every 6-12 months, and
> usually
> > >> abandoned once people figure out how difficult they are to remove
> > > properly.
> > >> I do think removing them *should* be a long term goal, but we
> really
> > > need
> > >> something more immediately actionable. Otherwise, nothing’s going
> to
> > >> happen, and we’ll be having this discussion again in a year or so
> > >> when
> > >> 

Re: Code quality, principles and rules

[Jeremy Hanna]

https://issues.apache.org/jira/browse/CASSANDRA-7837 may be some interesting 
context regarding what's been worked on to get rid of singletons and static 
initialization.

> On Mar 17, 2017, at 4:47 PM, Jonathan Haddad  wrote:
> 
> I'd like to think that if someone refactors existing code, making it more
> testable (with tests, of course) it should be acceptable on it's own
> merit.  In fact, in my opinion it sometimes makes more sense to do these
> types of refactorings for the sole purpose of improving stability and
> testability as opposed to mixing them with features.
> 
> You referenced the issue I fixed in one of the early emails.  The fix
> itself was a couple lines of code.  Refactoring the codebase to make it
> testable would have been a huge effort.  I wish I had time to do it.  I
> created CASSANDRA-13007 as a follow up with the intent of working on
> compaction from a purely architectural standpoint.  I think this type of
> thing should be done throughout the codebase.
> 
> Removing the singletons is a good first step, my vote is we just rip off
> the bandaid, do it, and move forward.
> 
> On Fri, Mar 17, 2017 at 2:20 PM Edward Capriolo 
> wrote:
> 
>>> On Fri, Mar 17, 2017 at 2:31 PM, Jason Brown  wrote:
>>> 
>>> To François's point about code coverage for new code, I think this makes
>> a
>>> lot of sense wrt large features (like the current work on
>> 8457/12229/9754).
>>> It's much simpler to (mentally, at least) isolate those changed sections
>>> and it'll show up better in a code coverage report. With small patches,
>>> that might be harder to achieve - however, as the patch should come with
>>> *some* tests (unless it's a truly trivial patch), it might just work
>> itself
>>> out.
>>> 
>>> On Fri, Mar 17, 2017 at 11:19 AM, Jason Brown 
>>> wrote:
>>> 
 As someone who spent a lot of time looking at the singletons topic in
>> the
 past, Blake brings a great perspective here. Figuring out and
>>> communicating
 how best to test with the system we have (and of course incrementally
 making that system easier to work with/test) seems like an achievable
>>> goal.
 
 On Fri, Mar 17, 2017 at 10:17 AM, Edward Capriolo <
>> edlinuxg...@gmail.com
 
 wrote:
 
> On Fri, Mar 17, 2017 at 12:33 PM, Blake Eggleston <
>> beggles...@apple.com
 
> wrote:
> 
>> I think we’re getting a little ahead of ourselves talking about DI
>> frameworks. Before that even becomes something worth talking about,
>>> we’d
>> need to have made serious progress on un-spaghettifying Cassandra in
>>> the
>> first place. It’s an extremely tall order. Adding a DI framework
>> right
> now
>> would be like throwing gasoline on a raging tire fire.
>> 
>> Removing singletons seems to come up every 6-12 months, and usually
>> abandoned once people figure out how difficult they are to remove
> properly.
>> I do think removing them *should* be a long term goal, but we really
> need
>> something more immediately actionable. Otherwise, nothing’s going to
>> happen, and we’ll be having this discussion again in a year or so
>> when
>> everyone’s angry that Cassandra 5.0 still isn’t ready for
>> production,
>>> a
>> year after it’s release.
>> 
>> That said, the reason singletons regularly get brought up is because
> doing
>> extensive testing of anything in Cassandra is pretty much
>> impossible,
> since
>> the code is basically this big web of interconnected global state.
> Testing
>> anything in isolation can’t be done, which, for a distributed
>>> database,
> is
>> crazy. It’s a chronic problem that handicaps our ability to release
>> a
>> stable database.
>> 
>> At this point, I think a more pragmatic approach would be to draft
>> and
>> enforce some coding standards that can be applied in day to day
> development
>> that drive incremental improvement of the testing and testability of
>>> the
>> project. What should be tested, how it should be tested. How to
>> write
> new
>> code that talks to the rest of Cassandra and is testable. How to fix
> bugs
>> in old code in a way that’s testable. We should also have some
> guidelines
>> around refactoring the wildly untested sections, how to get started,
> what
>> to do, what not to do, etc.
>> 
>> Thoughts?
> 
> 
> To make the conversation practical. There is one class I personally
>>> really
> want to refactor so it can be tested:
> 
> https://github.com/apache/cassandra/blob/trunk/src/java/org/
> apache/cassandra/net/OutboundTcpConnection.java
> 
> There is little coverage here. Questions like:
> what errors cause the connection to restart?
> when are undropable messages are dropped?
> what happens when the queue fills up?
> Infamous 

Re: Code quality, principles and rules

[Jonathan Haddad]

I'd like to think that if someone refactors existing code, making it more
testable (with tests, of course) it should be acceptable on it's own
merit.  In fact, in my opinion it sometimes makes more sense to do these
types of refactorings for the sole purpose of improving stability and
testability as opposed to mixing them with features.

You referenced the issue I fixed in one of the early emails.  The fix
itself was a couple lines of code.  Refactoring the codebase to make it
testable would have been a huge effort.  I wish I had time to do it.  I
created CASSANDRA-13007 as a follow up with the intent of working on
compaction from a purely architectural standpoint.  I think this type of
thing should be done throughout the codebase.

Removing the singletons is a good first step, my vote is we just rip off
the bandaid, do it, and move forward.

On Fri, Mar 17, 2017 at 2:20 PM Edward Capriolo 
wrote:

> On Fri, Mar 17, 2017 at 2:31 PM, Jason Brown  wrote:
>
> > To François's point about code coverage for new code, I think this makes
> a
> > lot of sense wrt large features (like the current work on
> 8457/12229/9754).
> > It's much simpler to (mentally, at least) isolate those changed sections
> > and it'll show up better in a code coverage report. With small patches,
> > that might be harder to achieve - however, as the patch should come with
> > *some* tests (unless it's a truly trivial patch), it might just work
> itself
> > out.
> >
> > On Fri, Mar 17, 2017 at 11:19 AM, Jason Brown 
> > wrote:
> >
> > > As someone who spent a lot of time looking at the singletons topic in
> the
> > > past, Blake brings a great perspective here. Figuring out and
> > communicating
> > > how best to test with the system we have (and of course incrementally
> > > making that system easier to work with/test) seems like an achievable
> > goal.
> > >
> > > On Fri, Mar 17, 2017 at 10:17 AM, Edward Capriolo <
> edlinuxg...@gmail.com
> > >
> > > wrote:
> > >
> > >> On Fri, Mar 17, 2017 at 12:33 PM, Blake Eggleston <
> beggles...@apple.com
> > >
> > >> wrote:
> > >>
> > >> > I think we’re getting a little ahead of ourselves talking about DI
> > >> > frameworks. Before that even becomes something worth talking about,
> > we’d
> > >> > need to have made serious progress on un-spaghettifying Cassandra in
> > the
> > >> > first place. It’s an extremely tall order. Adding a DI framework
> right
> > >> now
> > >> > would be like throwing gasoline on a raging tire fire.
> > >> >
> > >> > Removing singletons seems to come up every 6-12 months, and usually
> > >> > abandoned once people figure out how difficult they are to remove
> > >> properly.
> > >> > I do think removing them *should* be a long term goal, but we really
> > >> need
> > >> > something more immediately actionable. Otherwise, nothing’s going to
> > >> > happen, and we’ll be having this discussion again in a year or so
> when
> > >> > everyone’s angry that Cassandra 5.0 still isn’t ready for
> production,
> > a
> > >> > year after it’s release.
> > >> >
> > >> > That said, the reason singletons regularly get brought up is because
> > >> doing
> > >> > extensive testing of anything in Cassandra is pretty much
> impossible,
> > >> since
> > >> > the code is basically this big web of interconnected global state.
> > >> Testing
> > >> > anything in isolation can’t be done, which, for a distributed
> > database,
> > >> is
> > >> > crazy. It’s a chronic problem that handicaps our ability to release
> a
> > >> > stable database.
> > >> >
> > >> > At this point, I think a more pragmatic approach would be to draft
> and
> > >> > enforce some coding standards that can be applied in day to day
> > >> development
> > >> > that drive incremental improvement of the testing and testability of
> > the
> > >> > project. What should be tested, how it should be tested. How to
> write
> > >> new
> > >> > code that talks to the rest of Cassandra and is testable. How to fix
> > >> bugs
> > >> > in old code in a way that’s testable. We should also have some
> > >> guidelines
> > >> > around refactoring the wildly untested sections, how to get started,
> > >> what
> > >> > to do, what not to do, etc.
> > >> >
> > >> > Thoughts?
> > >>
> > >>
> > >> To make the conversation practical. There is one class I personally
> > really
> > >> want to refactor so it can be tested:
> > >>
> > >> https://github.com/apache/cassandra/blob/trunk/src/java/org/
> > >> apache/cassandra/net/OutboundTcpConnection.java
> > >>
> > >> There is little coverage here. Questions like:
> > >> what errors cause the connection to restart?
> > >> when are undropable messages are dropped?
> > >> what happens when the queue fills up?
> > >> Infamous throw new AssertionError(ex); (which probably bubble up to
> > >> nowhere)
> > >> what does the COALESCED strategy do in case XYZ.
> > >> A nifty label (wow a label you just never see those much!)
> > >> outer:
> > >> 

Re: Code quality, principles and rules

[Edward Capriolo]

On Fri, Mar 17, 2017 at 2:31 PM, Jason Brown  wrote:

> To François's point about code coverage for new code, I think this makes a
> lot of sense wrt large features (like the current work on 8457/12229/9754).
> It's much simpler to (mentally, at least) isolate those changed sections
> and it'll show up better in a code coverage report. With small patches,
> that might be harder to achieve - however, as the patch should come with
> *some* tests (unless it's a truly trivial patch), it might just work itself
> out.
>
> On Fri, Mar 17, 2017 at 11:19 AM, Jason Brown 
> wrote:
>
> > As someone who spent a lot of time looking at the singletons topic in the
> > past, Blake brings a great perspective here. Figuring out and
> communicating
> > how best to test with the system we have (and of course incrementally
> > making that system easier to work with/test) seems like an achievable
> goal.
> >
> > On Fri, Mar 17, 2017 at 10:17 AM, Edward Capriolo  >
> > wrote:
> >
> >> On Fri, Mar 17, 2017 at 12:33 PM, Blake Eggleston  >
> >> wrote:
> >>
> >> > I think we’re getting a little ahead of ourselves talking about DI
> >> > frameworks. Before that even becomes something worth talking about,
> we’d
> >> > need to have made serious progress on un-spaghettifying Cassandra in
> the
> >> > first place. It’s an extremely tall order. Adding a DI framework right
> >> now
> >> > would be like throwing gasoline on a raging tire fire.
> >> >
> >> > Removing singletons seems to come up every 6-12 months, and usually
> >> > abandoned once people figure out how difficult they are to remove
> >> properly.
> >> > I do think removing them *should* be a long term goal, but we really
> >> need
> >> > something more immediately actionable. Otherwise, nothing’s going to
> >> > happen, and we’ll be having this discussion again in a year or so when
> >> > everyone’s angry that Cassandra 5.0 still isn’t ready for production,
> a
> >> > year after it’s release.
> >> >
> >> > That said, the reason singletons regularly get brought up is because
> >> doing
> >> > extensive testing of anything in Cassandra is pretty much impossible,
> >> since
> >> > the code is basically this big web of interconnected global state.
> >> Testing
> >> > anything in isolation can’t be done, which, for a distributed
> database,
> >> is
> >> > crazy. It’s a chronic problem that handicaps our ability to release a
> >> > stable database.
> >> >
> >> > At this point, I think a more pragmatic approach would be to draft and
> >> > enforce some coding standards that can be applied in day to day
> >> development
> >> > that drive incremental improvement of the testing and testability of
> the
> >> > project. What should be tested, how it should be tested. How to write
> >> new
> >> > code that talks to the rest of Cassandra and is testable. How to fix
> >> bugs
> >> > in old code in a way that’s testable. We should also have some
> >> guidelines
> >> > around refactoring the wildly untested sections, how to get started,
> >> what
> >> > to do, what not to do, etc.
> >> >
> >> > Thoughts?
> >>
> >>
> >> To make the conversation practical. There is one class I personally
> really
> >> want to refactor so it can be tested:
> >>
> >> https://github.com/apache/cassandra/blob/trunk/src/java/org/
> >> apache/cassandra/net/OutboundTcpConnection.java
> >>
> >> There is little coverage here. Questions like:
> >> what errors cause the connection to restart?
> >> when are undropable messages are dropped?
> >> what happens when the queue fills up?
> >> Infamous throw new AssertionError(ex); (which probably bubble up to
> >> nowhere)
> >> what does the COALESCED strategy do in case XYZ.
> >> A nifty label (wow a label you just never see those much!)
> >> outer:
> >> while (!isStopped)
> >>
> >> Comments to jira's that probably are not explicitly tested:
> >> // If we haven't retried this message yet, put it back on the queue to
> >> retry after re-connecting.
> >> // See CASSANDRA-5393 and CASSANDRA-12192.
> >>
> >> If I were to undertake this cleanup, would there actually be support? IE
> >> if
> >> this going to turn into an "it aint broken. don't fix it thing" or a "we
> >> don't want to change stuff just to add tests" . Like will someone pledge
> >> to
> >> agree its kinda wonky and merge the effort in < 1 years time?
> >>
> >
> >
>

So ...:) If open a ticket to refactor OutboundTcpConnection.java to do
specific unit testing and possibly even pull things out to the point that I
can actually open a socket and to an end to end test will you/anyone
support that? (it sounds like your saying I must/should make a large
feature to add a test)

Re: Code quality, principles and rules

[benjamin roth]

I think you can refactor any project with little risk and increase test
coverage.
What is needed:
Rules. Discipline. Perseverance. Small iterations. Small iterations. Small
iterations.

   - Refactor in the smallest possible unit
   - Split large classes into smaller ones. Remove god classes by pulling
   out single methods or aspects. Maybe factor out method by method.
   - Maintain compatibility. Build facades, adapters, proxy objects for
   compatibility during refactoring process. Do not break interfaces if not
   really necessary or risky.
   - Push states into corners. E.g. when refactoring a single method, pass
   global state as parameter. So this single method becomes testable.

If you iterate like this maybe 1000 times, you will most likely break much
fewer things than doing a big bang refactor. You make code testable in
small steps.

Global state is the biggest disease, history of programming has ever seen.
Singletons are also not supergreat to test and static methods should be
avoided at all costs if they contain state.
Tested idempotent static methods should not be a problem.

>From my experience, you don't need a bloated DI framework to make a class
testable that depends somehow on static methods or singletons.
You just have to push the bad guys into a corner where they don't harm and
can be killed without risk in the very end.
E.g. instead of calling SomeClass.instance.doWhatEver() spread here and
there it can be encapsulated in a single method like
TestableClass.doWhatever() {SomeClass.instance.doWhatEver()}
Or the whole singleton is retrieved through TestableClass.getSomeClass().
So you can either mock the hell out of it or you inject a non-singleton
instance of that class at test-runtime.


2017-03-17 19:19 GMT+01:00 Jason Brown :

> As someone who spent a lot of time looking at the singletons topic in the
> past, Blake brings a great perspective here. Figuring out and communicating
> how best to test with the system we have (and of course incrementally
> making that system easier to work with/test) seems like an achievable goal.
>
> On Fri, Mar 17, 2017 at 10:17 AM, Edward Capriolo 
> wrote:
>
> > On Fri, Mar 17, 2017 at 12:33 PM, Blake Eggleston 
> > wrote:
> >
> > > I think we’re getting a little ahead of ourselves talking about DI
> > > frameworks. Before that even becomes something worth talking about,
> we’d
> > > need to have made serious progress on un-spaghettifying Cassandra in
> the
> > > first place. It’s an extremely tall order. Adding a DI framework right
> > now
> > > would be like throwing gasoline on a raging tire fire.
> > >
> > > Removing singletons seems to come up every 6-12 months, and usually
> > > abandoned once people figure out how difficult they are to remove
> > properly.
> > > I do think removing them *should* be a long term goal, but we really
> need
> > > something more immediately actionable. Otherwise, nothing’s going to
> > > happen, and we’ll be having this discussion again in a year or so when
> > > everyone’s angry that Cassandra 5.0 still isn’t ready for production, a
> > > year after it’s release.
> > >
> > > That said, the reason singletons regularly get brought up is because
> > doing
> > > extensive testing of anything in Cassandra is pretty much impossible,
> > since
> > > the code is basically this big web of interconnected global state.
> > Testing
> > > anything in isolation can’t be done, which, for a distributed database,
> > is
> > > crazy. It’s a chronic problem that handicaps our ability to release a
> > > stable database.
> > >
> > > At this point, I think a more pragmatic approach would be to draft and
> > > enforce some coding standards that can be applied in day to day
> > development
> > > that drive incremental improvement of the testing and testability of
> the
> > > project. What should be tested, how it should be tested. How to write
> new
> > > code that talks to the rest of Cassandra and is testable. How to fix
> bugs
> > > in old code in a way that’s testable. We should also have some
> guidelines
> > > around refactoring the wildly untested sections, how to get started,
> what
> > > to do, what not to do, etc.
> > >
> > > Thoughts?
> >
> >
> > To make the conversation practical. There is one class I personally
> really
> > want to refactor so it can be tested:
> >
> > https://github.com/apache/cassandra/blob/trunk/src/java/
> > org/apache/cassandra/net/OutboundTcpConnection.java
> >
> > There is little coverage here. Questions like:
> > what errors cause the connection to restart?
> > when are undropable messages are dropped?
> > what happens when the queue fills up?
> > Infamous throw new AssertionError(ex); (which probably bubble up to
> > nowhere)
> > what does the COALESCED strategy do in case XYZ.
> > A nifty label (wow a label you just never see those much!)
> > outer:
> > while (!isStopped)
> >
> > Comments to jira's that probably are not explicitly 

Re: Code quality, principles and rules

[Jason Brown]

To François's point about code coverage for new code, I think this makes a
lot of sense wrt large features (like the current work on 8457/12229/9754).
It's much simpler to (mentally, at least) isolate those changed sections
and it'll show up better in a code coverage report. With small patches,
that might be harder to achieve - however, as the patch should come with
*some* tests (unless it's a truly trivial patch), it might just work itself
out.

On Fri, Mar 17, 2017 at 11:19 AM, Jason Brown  wrote:

> As someone who spent a lot of time looking at the singletons topic in the
> past, Blake brings a great perspective here. Figuring out and communicating
> how best to test with the system we have (and of course incrementally
> making that system easier to work with/test) seems like an achievable goal.
>
> On Fri, Mar 17, 2017 at 10:17 AM, Edward Capriolo 
> wrote:
>
>> On Fri, Mar 17, 2017 at 12:33 PM, Blake Eggleston 
>> wrote:
>>
>> > I think we’re getting a little ahead of ourselves talking about DI
>> > frameworks. Before that even becomes something worth talking about, we’d
>> > need to have made serious progress on un-spaghettifying Cassandra in the
>> > first place. It’s an extremely tall order. Adding a DI framework right
>> now
>> > would be like throwing gasoline on a raging tire fire.
>> >
>> > Removing singletons seems to come up every 6-12 months, and usually
>> > abandoned once people figure out how difficult they are to remove
>> properly.
>> > I do think removing them *should* be a long term goal, but we really
>> need
>> > something more immediately actionable. Otherwise, nothing’s going to
>> > happen, and we’ll be having this discussion again in a year or so when
>> > everyone’s angry that Cassandra 5.0 still isn’t ready for production, a
>> > year after it’s release.
>> >
>> > That said, the reason singletons regularly get brought up is because
>> doing
>> > extensive testing of anything in Cassandra is pretty much impossible,
>> since
>> > the code is basically this big web of interconnected global state.
>> Testing
>> > anything in isolation can’t be done, which, for a distributed database,
>> is
>> > crazy. It’s a chronic problem that handicaps our ability to release a
>> > stable database.
>> >
>> > At this point, I think a more pragmatic approach would be to draft and
>> > enforce some coding standards that can be applied in day to day
>> development
>> > that drive incremental improvement of the testing and testability of the
>> > project. What should be tested, how it should be tested. How to write
>> new
>> > code that talks to the rest of Cassandra and is testable. How to fix
>> bugs
>> > in old code in a way that’s testable. We should also have some
>> guidelines
>> > around refactoring the wildly untested sections, how to get started,
>> what
>> > to do, what not to do, etc.
>> >
>> > Thoughts?
>>
>>
>> To make the conversation practical. There is one class I personally really
>> want to refactor so it can be tested:
>>
>> https://github.com/apache/cassandra/blob/trunk/src/java/org/
>> apache/cassandra/net/OutboundTcpConnection.java
>>
>> There is little coverage here. Questions like:
>> what errors cause the connection to restart?
>> when are undropable messages are dropped?
>> what happens when the queue fills up?
>> Infamous throw new AssertionError(ex); (which probably bubble up to
>> nowhere)
>> what does the COALESCED strategy do in case XYZ.
>> A nifty label (wow a label you just never see those much!)
>> outer:
>> while (!isStopped)
>>
>> Comments to jira's that probably are not explicitly tested:
>> // If we haven't retried this message yet, put it back on the queue to
>> retry after re-connecting.
>> // See CASSANDRA-5393 and CASSANDRA-12192.
>>
>> If I were to undertake this cleanup, would there actually be support? IE
>> if
>> this going to turn into an "it aint broken. don't fix it thing" or a "we
>> don't want to change stuff just to add tests" . Like will someone pledge
>> to
>> agree its kinda wonky and merge the effort in < 1 years time?
>>
>
>

Re: Code quality, principles and rules

[Blake Eggleston]

I think we’re getting a little ahead of ourselves talking about DI frameworks. 
Before that even becomes something worth talking about, we’d need to have made 
serious progress on un-spaghettifying Cassandra in the first place. It’s an 
extremely tall order. Adding a DI framework right now would be like throwing 
gasoline on a raging tire fire.

Removing singletons seems to come up every 6-12 months, and usually abandoned 
once people figure out how difficult they are to remove properly. I do think 
removing them *should* be a long term goal, but we really need something more 
immediately actionable. Otherwise, nothing’s going to happen, and we’ll be 
having this discussion again in a year or so when everyone’s angry that 
Cassandra 5.0 still isn’t ready for production, a year after it’s release.

That said, the reason singletons regularly get brought up is because doing 
extensive testing of anything in Cassandra is pretty much impossible, since the 
code is basically this big web of interconnected global state. Testing anything 
in isolation can’t be done, which, for a distributed database, is crazy. It’s a 
chronic problem that handicaps our ability to release a stable database.

At this point, I think a more pragmatic approach would be to draft and enforce 
some coding standards that can be applied in day to day development that drive 
incremental improvement of the testing and testability of the project. What 
should be tested, how it should be tested. How to write new code that talks to 
the rest of Cassandra and is testable. How to fix bugs in old code in a way 
that’s testable. We should also have some guidelines around refactoring the 
wildly untested sections, how to get started, what to do, what not to do, etc.

Thoughts?

Re: Code quality, principles and rules

[Edward Capriolo]

On Fri, Mar 17, 2017 at 6:41 AM, Ryan Svihla  wrote:

> Different DI frameworks have different initialization costs, even inside of
> spring even depending on how you wire up dependencies (did it use autowire
> with reflection, parse a giant XML of explicit dependencies, etc).
>
> To back this assertion up for awhile in that community benching different
> DI frameworks perf was a thing and you can find benchmarks galore with a
> quick Google.
>
> The practical cost is also dependent on the lifecycles used (transient
> versus Singleton style for example) and features used (Interceptors
> depending on implementation can get expensive).
>
> So I think there should be some quantification of cost before a framework
> is considered, something like dagger2 which uses codegen I wager is only a
> cost at compile time (have not benched it, but looking at it's feature set,
> that's my guess) , Spring I know from experience even with the most optimal
> settings is slower on initialization time than doing by DI "by hand" at
> minimum, and that can sometimes be substantial.
>
>
> On Mar 17, 2017 12:29 AM, "Edward Capriolo"  wrote:
>
> On Thu, Mar 16, 2017 at 5:18 PM, Jason Brown  wrote:
>
> > >> do we have plan to integrate with a dependency injection framework?
> >
> > No, we (the maintainers) have been pretty much against more frameworks
> due
> > to performance reasons, overhead, and dependency management problems.
> >
> > On Thu, Mar 16, 2017 at 2:04 PM, Qingcun Zhou 
> > wrote:
> >
> > > Since we're here, do we have plan to integrate with a dependency
> > injection
> > > framework like Dagger2? Otherwise it'll be difficult to write unit test
> > > cases.
> > >
> > > On Thu, Mar 16, 2017 at 1:16 PM, Edward Capriolo <
> edlinuxg...@gmail.com>
> > > wrote:
> > >
> > > > On Thu, Mar 16, 2017 at 3:10 PM, Jeff Jirsa 
> wrote:
> > > >
> > > > >
> > > > >
> > > > > On 2017-03-16 10:32 (-0700), François Deliège <
> > franc...@instagram.com>
> > > > > wrote:
> > > > > >
> > > > > > To get this started, here is an initial proposal:
> > > > > >
> > > > > > Principles:
> > > > > >
> > > > > > 1. Tests always pass.  This is the starting point. If we don't
> care
> > > > > about test failures, then we should stop writing tests. A recurring
> > > > failing
> > > > > test carries no signal and is better deleted.
> > > > > > 2. The code is tested.
> > > > > >
> > > > > > Assuming we can align on these principles, here is a proposal for
> > > their
> > > > > implementation.
> > > > > >
> > > > > > Rules:
> > > > > >
> > > > > > 1. Each new release passes all tests (no flakinesss).
> > > > > > 2. If a patch has a failing test (test touching the same code
> > path),
> > > > the
> > > > > code or test should be fixed prior to being accepted.
> > > > > > 3. Bugs fixes should have one test that fails prior to the fix
> and
> > > > > passes after fix.
> > > > > > 4. New code should have at least 90% test coverage.
> > > > > >
> > > > > First I was
> > > > > I agree with all of these and hope they become codified and
> > followed. I
> > > > > don't know anyone who believes we should be committing code that
> > breaks
> > > > > tests - but we should be more strict with requiring green test
> runs,
> > > and
> > > > > perhaps more strict with reverting patches that break tests (or
> cause
> > > > them
> > > > > to be flakey).
> > > > >
> > > > > Ed also noted on the user list [0] that certain sections of the
> code
> > > > > itself are difficult to test because of singletons - I agree with
> the
> > > > > suggestion that it's time to revisit CASSANDRA-7837 and
> > CASSANDRA-10283
> > > > >
> > > > > Finally, we should also recall Jason's previous notes [1] that the
> > > actual
> > > > > test infrastructure available is limited - the system provided by
> > > > Datastax
> > > > > is not generally open to everyone (and not guaranteed to be
> > permanent),
> > > > and
> > > > > the infrastructure currently available to the ASF is somewhat
> limited
> > > > (much
> > > > > slower, at the very least). If we require tests passing (and I
> agree
> > > that
> > > > > we should), we need to define how we're going to be testing (or how
> > > we're
> > > > > going to be sharing test results), because the ASF hardware isn't
> > going
> > > > to
> > > > > be able to do dozens of dev branch dtest runs per day in its
> current
> > > > form.
> > > > >
> > > > > 0: https://lists.apache.org/thread.html/
> > f6f3fc6d0ad1bd54a6185ce7bd7a2f
> > > > > 6f09759a02352ffc05df92eef6@%3Cuser.cassandra.apache.org%3E
> > > > > 1: https://lists.apache.org/thread.html/
> > 5fb8f0446ab97644100e4ef987f36e
> > > > > 07f44e8dd6d38f5dc81ecb3cdd@%3Cdev.cassandra.apache.org%3E
> > > > >
> > > > >
> > > > >
> > > > Ed also noted on the user list [0] that certain sections of the code
> > > itself
> > > > are difficult to test because of singletons - I agree with the
> > 

Re: Code quality, principles and rules

[DuyHai Doan]

"Otherwise it'll be difficult to write unit test cases."

Having to pull in dependency injection framework to make unit testing
easier is generally a sign of code design issue.

With constructor injection & other techniques, there is more than enough to
unit test your code without having to pull external libs

On Thu, Mar 16, 2017 at 10:18 PM, Jason Brown  wrote:

> >> do we have plan to integrate with a dependency injection framework?
>
> No, we (the maintainers) have been pretty much against more frameworks due
> to performance reasons, overhead, and dependency management problems.
>
> On Thu, Mar 16, 2017 at 2:04 PM, Qingcun Zhou 
> wrote:
>
> > Since we're here, do we have plan to integrate with a dependency
> injection
> > framework like Dagger2? Otherwise it'll be difficult to write unit test
> > cases.
> >
> > On Thu, Mar 16, 2017 at 1:16 PM, Edward Capriolo 
> > wrote:
> >
> > > On Thu, Mar 16, 2017 at 3:10 PM, Jeff Jirsa  wrote:
> > >
> > > >
> > > >
> > > > On 2017-03-16 10:32 (-0700), François Deliège <
> franc...@instagram.com>
> > > > wrote:
> > > > >
> > > > > To get this started, here is an initial proposal:
> > > > >
> > > > > Principles:
> > > > >
> > > > > 1. Tests always pass.  This is the starting point. If we don't care
> > > > about test failures, then we should stop writing tests. A recurring
> > > failing
> > > > test carries no signal and is better deleted.
> > > > > 2. The code is tested.
> > > > >
> > > > > Assuming we can align on these principles, here is a proposal for
> > their
> > > > implementation.
> > > > >
> > > > > Rules:
> > > > >
> > > > > 1. Each new release passes all tests (no flakinesss).
> > > > > 2. If a patch has a failing test (test touching the same code
> path),
> > > the
> > > > code or test should be fixed prior to being accepted.
> > > > > 3. Bugs fixes should have one test that fails prior to the fix and
> > > > passes after fix.
> > > > > 4. New code should have at least 90% test coverage.
> > > > >
> > > > First I was
> > > > I agree with all of these and hope they become codified and
> followed. I
> > > > don't know anyone who believes we should be committing code that
> breaks
> > > > tests - but we should be more strict with requiring green test runs,
> > and
> > > > perhaps more strict with reverting patches that break tests (or cause
> > > them
> > > > to be flakey).
> > > >
> > > > Ed also noted on the user list [0] that certain sections of the code
> > > > itself are difficult to test because of singletons - I agree with the
> > > > suggestion that it's time to revisit CASSANDRA-7837 and
> CASSANDRA-10283
> > > >
> > > > Finally, we should also recall Jason's previous notes [1] that the
> > actual
> > > > test infrastructure available is limited - the system provided by
> > > Datastax
> > > > is not generally open to everyone (and not guaranteed to be
> permanent),
> > > and
> > > > the infrastructure currently available to the ASF is somewhat limited
> > > (much
> > > > slower, at the very least). If we require tests passing (and I agree
> > that
> > > > we should), we need to define how we're going to be testing (or how
> > we're
> > > > going to be sharing test results), because the ASF hardware isn't
> going
> > > to
> > > > be able to do dozens of dev branch dtest runs per day in its current
> > > form.
> > > >
> > > > 0: https://lists.apache.org/thread.html/
> f6f3fc6d0ad1bd54a6185ce7bd7a2f
> > > > 6f09759a02352ffc05df92eef6@%3Cuser.cassandra.apache.org%3E
> > > > 1: https://lists.apache.org/thread.html/
> 5fb8f0446ab97644100e4ef987f36e
> > > > 07f44e8dd6d38f5dc81ecb3cdd@%3Cdev.cassandra.apache.org%3E
> > > >
> > > >
> > > >
> > > Ed also noted on the user list [0] that certain sections of the code
> > itself
> > > are difficult to test because of singletons - I agree with the
> suggestion
> > > that it's time to revisit CASSANDRA-7837 and CASSANDRA-10283
> > >
> > > Thanks for the shout out!
> > >
> > > I was just looking at a patch about compaction. The patch was to
> > calculate
> > > free space correctly in case X. Compaction is not something that
> requires
> > > multiple nodes to test. The logic on the surface seems simple: find
> > tables
> > > of similar size and select them and merge them. The reality is it turns
> > out
> > > now to be that way. The coverage itself both branch and line may be
> very
> > > high, but what the code does not do is directly account for a wide
> > variety
> > > of scenarios. Without direct tests you end up with a mental
> approximation
> > > of what it does and that varies from person to person and accounts for
> > the
> > > cases that fit in your mind. For example, you personally are only
> running
> > > LevelDB inspired compaction.
> > >
> > > Being that this this is not a multi-node problem you should be able to
> re
> > > factor this heavily. Pulling everything out to a static method where
> all
> > > the 

Re: Code quality, principles and rules

[Jason Brown]

>> do we have plan to integrate with a dependency injection framework?

No, we (the maintainers) have been pretty much against more frameworks due
to performance reasons, overhead, and dependency management problems.

On Thu, Mar 16, 2017 at 2:04 PM, Qingcun Zhou  wrote:

> Since we're here, do we have plan to integrate with a dependency injection
> framework like Dagger2? Otherwise it'll be difficult to write unit test
> cases.
>
> On Thu, Mar 16, 2017 at 1:16 PM, Edward Capriolo 
> wrote:
>
> > On Thu, Mar 16, 2017 at 3:10 PM, Jeff Jirsa  wrote:
> >
> > >
> > >
> > > On 2017-03-16 10:32 (-0700), François Deliège 
> > > wrote:
> > > >
> > > > To get this started, here is an initial proposal:
> > > >
> > > > Principles:
> > > >
> > > > 1. Tests always pass.  This is the starting point. If we don't care
> > > about test failures, then we should stop writing tests. A recurring
> > failing
> > > test carries no signal and is better deleted.
> > > > 2. The code is tested.
> > > >
> > > > Assuming we can align on these principles, here is a proposal for
> their
> > > implementation.
> > > >
> > > > Rules:
> > > >
> > > > 1. Each new release passes all tests (no flakinesss).
> > > > 2. If a patch has a failing test (test touching the same code path),
> > the
> > > code or test should be fixed prior to being accepted.
> > > > 3. Bugs fixes should have one test that fails prior to the fix and
> > > passes after fix.
> > > > 4. New code should have at least 90% test coverage.
> > > >
> > > First I was
> > > I agree with all of these and hope they become codified and followed. I
> > > don't know anyone who believes we should be committing code that breaks
> > > tests - but we should be more strict with requiring green test runs,
> and
> > > perhaps more strict with reverting patches that break tests (or cause
> > them
> > > to be flakey).
> > >
> > > Ed also noted on the user list [0] that certain sections of the code
> > > itself are difficult to test because of singletons - I agree with the
> > > suggestion that it's time to revisit CASSANDRA-7837 and CASSANDRA-10283
> > >
> > > Finally, we should also recall Jason's previous notes [1] that the
> actual
> > > test infrastructure available is limited - the system provided by
> > Datastax
> > > is not generally open to everyone (and not guaranteed to be permanent),
> > and
> > > the infrastructure currently available to the ASF is somewhat limited
> > (much
> > > slower, at the very least). If we require tests passing (and I agree
> that
> > > we should), we need to define how we're going to be testing (or how
> we're
> > > going to be sharing test results), because the ASF hardware isn't going
> > to
> > > be able to do dozens of dev branch dtest runs per day in its current
> > form.
> > >
> > > 0: https://lists.apache.org/thread.html/f6f3fc6d0ad1bd54a6185ce7bd7a2f
> > > 6f09759a02352ffc05df92eef6@%3Cuser.cassandra.apache.org%3E
> > > 1: https://lists.apache.org/thread.html/5fb8f0446ab97644100e4ef987f36e
> > > 07f44e8dd6d38f5dc81ecb3cdd@%3Cdev.cassandra.apache.org%3E
> > >
> > >
> > >
> > Ed also noted on the user list [0] that certain sections of the code
> itself
> > are difficult to test because of singletons - I agree with the suggestion
> > that it's time to revisit CASSANDRA-7837 and CASSANDRA-10283
> >
> > Thanks for the shout out!
> >
> > I was just looking at a patch about compaction. The patch was to
> calculate
> > free space correctly in case X. Compaction is not something that requires
> > multiple nodes to test. The logic on the surface seems simple: find
> tables
> > of similar size and select them and merge them. The reality is it turns
> out
> > now to be that way. The coverage itself both branch and line may be very
> > high, but what the code does not do is directly account for a wide
> variety
> > of scenarios. Without direct tests you end up with a mental approximation
> > of what it does and that varies from person to person and accounts for
> the
> > cases that fit in your mind. For example, you personally are only running
> > LevelDB inspired compaction.
> >
> > Being that this this is not a multi-node problem you should be able to re
> > factor this heavily. Pulling everything out to a static method where all
> > the parameters are arguments, or inject a lot of mocks in the current
> code,
> > and develop some scenario based coverage.
> >
> > That is how i typically "rescue" code I take over. I look at the
> nightmare
> > and say, "damn i am really afraid to touch this". I construct 8 scenarios
> > that test green. Then I force some testing into it through careful re
> > factoring. Now, I probably know -something- about it. Now, you are fairly
> > free to do a wide ranging refactor, because you at least counted for 8
> > scenarios and you put unit test traps so that some rules are enforced.
> (Or
> > the person changing the code has to actively REMOVE 

Re: Code quality, principles and rules

[Qingcun Zhou]

Since we're here, do we have plan to integrate with a dependency injection
framework like Dagger2? Otherwise it'll be difficult to write unit test
cases.

On Thu, Mar 16, 2017 at 1:16 PM, Edward Capriolo 
wrote:

> On Thu, Mar 16, 2017 at 3:10 PM, Jeff Jirsa  wrote:
>
> >
> >
> > On 2017-03-16 10:32 (-0700), François Deliège 
> > wrote:
> > >
> > > To get this started, here is an initial proposal:
> > >
> > > Principles:
> > >
> > > 1. Tests always pass.  This is the starting point. If we don't care
> > about test failures, then we should stop writing tests. A recurring
> failing
> > test carries no signal and is better deleted.
> > > 2. The code is tested.
> > >
> > > Assuming we can align on these principles, here is a proposal for their
> > implementation.
> > >
> > > Rules:
> > >
> > > 1. Each new release passes all tests (no flakinesss).
> > > 2. If a patch has a failing test (test touching the same code path),
> the
> > code or test should be fixed prior to being accepted.
> > > 3. Bugs fixes should have one test that fails prior to the fix and
> > passes after fix.
> > > 4. New code should have at least 90% test coverage.
> > >
> > First I was
> > I agree with all of these and hope they become codified and followed. I
> > don't know anyone who believes we should be committing code that breaks
> > tests - but we should be more strict with requiring green test runs, and
> > perhaps more strict with reverting patches that break tests (or cause
> them
> > to be flakey).
> >
> > Ed also noted on the user list [0] that certain sections of the code
> > itself are difficult to test because of singletons - I agree with the
> > suggestion that it's time to revisit CASSANDRA-7837 and CASSANDRA-10283
> >
> > Finally, we should also recall Jason's previous notes [1] that the actual
> > test infrastructure available is limited - the system provided by
> Datastax
> > is not generally open to everyone (and not guaranteed to be permanent),
> and
> > the infrastructure currently available to the ASF is somewhat limited
> (much
> > slower, at the very least). If we require tests passing (and I agree that
> > we should), we need to define how we're going to be testing (or how we're
> > going to be sharing test results), because the ASF hardware isn't going
> to
> > be able to do dozens of dev branch dtest runs per day in its current
> form.
> >
> > 0: https://lists.apache.org/thread.html/f6f3fc6d0ad1bd54a6185ce7bd7a2f
> > 6f09759a02352ffc05df92eef6@%3Cuser.cassandra.apache.org%3E
> > 1: https://lists.apache.org/thread.html/5fb8f0446ab97644100e4ef987f36e
> > 07f44e8dd6d38f5dc81ecb3cdd@%3Cdev.cassandra.apache.org%3E
> >
> >
> >
> Ed also noted on the user list [0] that certain sections of the code itself
> are difficult to test because of singletons - I agree with the suggestion
> that it's time to revisit CASSANDRA-7837 and CASSANDRA-10283
>
> Thanks for the shout out!
>
> I was just looking at a patch about compaction. The patch was to calculate
> free space correctly in case X. Compaction is not something that requires
> multiple nodes to test. The logic on the surface seems simple: find tables
> of similar size and select them and merge them. The reality is it turns out
> now to be that way. The coverage itself both branch and line may be very
> high, but what the code does not do is directly account for a wide variety
> of scenarios. Without direct tests you end up with a mental approximation
> of what it does and that varies from person to person and accounts for the
> cases that fit in your mind. For example, you personally are only running
> LevelDB inspired compaction.
>
> Being that this this is not a multi-node problem you should be able to re
> factor this heavily. Pulling everything out to a static method where all
> the parameters are arguments, or inject a lot of mocks in the current code,
> and develop some scenario based coverage.
>
> That is how i typically "rescue" code I take over. I look at the nightmare
> and say, "damn i am really afraid to touch this". I construct 8 scenarios
> that test green. Then I force some testing into it through careful re
> factoring. Now, I probably know -something- about it. Now, you are fairly
> free to do a wide ranging refactor, because you at least counted for 8
> scenarios and you put unit test traps so that some rules are enforced. (Or
> the person changing the code has to actively REMOVE your tests asserting it
> was not or no longer is valid). Later on you (or someone else)  __STILL__
> might screw the entire thing up, but at least you can now build forward.
>
> Anyway that patch on compaction was great and I am sure it improved things.
> That being said it did not add any tests :). So it can easily be undone by
> the next person who does not understand the specific issue trying to be
> addressed. Inline comments almost scream to me 'we need a test' not
> everyone believes that.
>



-- 
Thank you 

Re: Code quality, principles and rules

[Edward Capriolo]

On Thu, Mar 16, 2017 at 3:10 PM, Jeff Jirsa  wrote:

>
>
> On 2017-03-16 10:32 (-0700), François Deliège 
> wrote:
> >
> > To get this started, here is an initial proposal:
> >
> > Principles:
> >
> > 1. Tests always pass.  This is the starting point. If we don't care
> about test failures, then we should stop writing tests. A recurring failing
> test carries no signal and is better deleted.
> > 2. The code is tested.
> >
> > Assuming we can align on these principles, here is a proposal for their
> implementation.
> >
> > Rules:
> >
> > 1. Each new release passes all tests (no flakinesss).
> > 2. If a patch has a failing test (test touching the same code path), the
> code or test should be fixed prior to being accepted.
> > 3. Bugs fixes should have one test that fails prior to the fix and
> passes after fix.
> > 4. New code should have at least 90% test coverage.
> >
> First I was
> I agree with all of these and hope they become codified and followed. I
> don't know anyone who believes we should be committing code that breaks
> tests - but we should be more strict with requiring green test runs, and
> perhaps more strict with reverting patches that break tests (or cause them
> to be flakey).
>
> Ed also noted on the user list [0] that certain sections of the code
> itself are difficult to test because of singletons - I agree with the
> suggestion that it's time to revisit CASSANDRA-7837 and CASSANDRA-10283
>
> Finally, we should also recall Jason's previous notes [1] that the actual
> test infrastructure available is limited - the system provided by Datastax
> is not generally open to everyone (and not guaranteed to be permanent), and
> the infrastructure currently available to the ASF is somewhat limited (much
> slower, at the very least). If we require tests passing (and I agree that
> we should), we need to define how we're going to be testing (or how we're
> going to be sharing test results), because the ASF hardware isn't going to
> be able to do dozens of dev branch dtest runs per day in its current form.
>
> 0: https://lists.apache.org/thread.html/f6f3fc6d0ad1bd54a6185ce7bd7a2f
> 6f09759a02352ffc05df92eef6@%3Cuser.cassandra.apache.org%3E
> 1: https://lists.apache.org/thread.html/5fb8f0446ab97644100e4ef987f36e
> 07f44e8dd6d38f5dc81ecb3cdd@%3Cdev.cassandra.apache.org%3E
>
>
>
Ed also noted on the user list [0] that certain sections of the code itself
are difficult to test because of singletons - I agree with the suggestion
that it's time to revisit CASSANDRA-7837 and CASSANDRA-10283

Thanks for the shout out!

I was just looking at a patch about compaction. The patch was to calculate
free space correctly in case X. Compaction is not something that requires
multiple nodes to test. The logic on the surface seems simple: find tables
of similar size and select them and merge them. The reality is it turns out
now to be that way. The coverage itself both branch and line may be very
high, but what the code does not do is directly account for a wide variety
of scenarios. Without direct tests you end up with a mental approximation
of what it does and that varies from person to person and accounts for the
cases that fit in your mind. For example, you personally are only running
LevelDB inspired compaction.

Being that this this is not a multi-node problem you should be able to re
factor this heavily. Pulling everything out to a static method where all
the parameters are arguments, or inject a lot of mocks in the current code,
and develop some scenario based coverage.

That is how i typically "rescue" code I take over. I look at the nightmare
and say, "damn i am really afraid to touch this". I construct 8 scenarios
that test green. Then I force some testing into it through careful re
factoring. Now, I probably know -something- about it. Now, you are fairly
free to do a wide ranging refactor, because you at least counted for 8
scenarios and you put unit test traps so that some rules are enforced. (Or
the person changing the code has to actively REMOVE your tests asserting it
was not or no longer is valid). Later on you (or someone else)  __STILL__
might screw the entire thing up, but at least you can now build forward.

Anyway that patch on compaction was great and I am sure it improved things.
That being said it did not add any tests :). So it can easily be undone by
the next person who does not understand the specific issue trying to be
addressed. Inline comments almost scream to me 'we need a test' not
everyone believes that.

Re: Code quality, principles and rules

[Jeff Jirsa]

On 2017-03-16 10:32 (-0700), François Deliège  wrote: 
> 
> To get this started, here is an initial proposal:
> 
> Principles:
> 
> 1. Tests always pass.  This is the starting point. If we don't care about 
> test failures, then we should stop writing tests. A recurring failing test 
> carries no signal and is better deleted.
> 2. The code is tested.
> 
> Assuming we can align on these principles, here is a proposal for their 
> implementation.
> 
> Rules:
> 
> 1. Each new release passes all tests (no flakinesss).
> 2. If a patch has a failing test (test touching the same code path), the code 
> or test should be fixed prior to being accepted.
> 3. Bugs fixes should have one test that fails prior to the fix and passes 
> after fix.
> 4. New code should have at least 90% test coverage.
> 

I agree with all of these and hope they become codified and followed. I don't 
know anyone who believes we should be committing code that breaks tests - but 
we should be more strict with requiring green test runs, and perhaps more 
strict with reverting patches that break tests (or cause them to be flakey). 

Ed also noted on the user list [0] that certain sections of the code itself are 
difficult to test because of singletons - I agree with the suggestion that it's 
time to revisit CASSANDRA-7837 and CASSANDRA-10283

Finally, we should also recall Jason's previous notes [1] that the actual test 
infrastructure available is limited - the system provided by Datastax is not 
generally open to everyone (and not guaranteed to be permanent), and the 
infrastructure currently available to the ASF is somewhat limited (much slower, 
at the very least). If we require tests passing (and I agree that we should), 
we need to define how we're going to be testing (or how we're going to be 
sharing test results), because the ASF hardware isn't going to be able to do 
dozens of dev branch dtest runs per day in its current form.  

0: 
https://lists.apache.org/thread.html/f6f3fc6d0ad1bd54a6185ce7bd7a2f6f09759a02352ffc05df92eef6@%3Cuser.cassandra.apache.org%3E
1: 
https://lists.apache.org/thread.html/5fb8f0446ab97644100e4ef987f36e07f44e8dd6d38f5dc81ecb3cdd@%3Cdev.cassandra.apache.org%3E
 

Code quality, principles and rules

[François Deliège]

Hi Dev,

What principles do we have? How do we implement them?

Our team has been evaluating 3.0.x and 3.x for a large production deployment.  
We have noticed broken tests and have been working on several patches.  
However, large parts of the code base are wildly untested, which makes new 
contributions more delicate.

All of this ultimately reduces our confidence in the new releases and slows 
down our adoption of the 3.0 / 3.x and future 4.0 releases.

So, I'd like to have a constructive discussion around 2 questions:

1. What principles should the community have in place about code quality and 
ensuring its long term productivity?
2. What are good implementationg (as in rules) of these principles?

To get this started, here is an initial proposal:

Principles:

1. Tests always pass.  This is the starting point. If we don't care about test 
failures, then we should stop writing tests. A recurring failing test carries 
no signal and is better deleted.
2. The code is tested.

Assuming we can align on these principles, here is a proposal for their 
implementation.

Rules:

1. Each new release passes all tests (no flakinesss).
2. If a patch has a failing test (test touching the same code path), the code 
or test should be fixed prior to being accepted.
3. Bugs fixes should have one test that fails prior to the fix and passes after 
fix.
4. New code should have at least 90% test coverage.

Looking forward to reading your feedback,

@fdeliege