Re: Question on assert
We can spend hours arguing about assert vs exceptions. I have seen it happen in every company I worked for. Overall, based on the patches I have reviewed, it seems to me that in general people are using them only has internal safety checks. Unfortunatly, the code change and we can miss things. If anybody think that some SPECIFIC assertions should be replaced by some real checks, I think the best way to do it is to open a JIRA ticket to raise the problem.
Re: Question on assert
As an aside, C* for some reason heavily uses asserts in unit tests, which adds to the "can't see the forest for the trees" problem. I see no reason for that. they should all be moved over to junit asserts. On 09/22/2016 03:52 AM, Benjamin Lerer wrote: We can spend hours arguing about assert vs exceptions. I have seen it happen in every company I worked for. Overall, based on the patches I have reviewed, it seems to me that in general people are using them only has internal safety checks. Unfortunatly, the code change and we can miss things. If anybody think that some SPECIFIC assertions should be replaced by some real checks, I think the best way to do it is to open a JIRA ticket to raise the problem.
Re: Question on assert
I fully agree. On Thu, Sep 22, 2016 at 11:57 AM, Dave Brosius wrote: > As an aside, C* for some reason heavily uses asserts in unit tests, which > adds to the "can't see the forest for the trees" problem. I see no reason > for that. they should all be moved over to junit asserts. > > > > On 09/22/2016 03:52 AM, Benjamin Lerer wrote: > >> We can spend hours arguing about assert vs exceptions. I have seen it >> happen in every company I worked for. >> Overall, based on the patches I have reviewed, it seems to me that in >> general people are using them only has internal safety checks. >> Unfortunatly, the code change and we can miss things. >> If anybody think that some SPECIFIC assertions should be replaced by some >> real checks, I think the best way to do it is to open a JIRA ticket to >> raise the problem. >> >> >
Failing tests 2016-09-22
Hi All, cassandra-3.9: === testall: 5 failures org.apache.cassandra.cql3.ViewFilteringTest .testMVCreationSelectRestrictions org.apache.cassandra.cql3.ViewFilteringTest .testClusteringKeyFilteringRestrictions org.apache.cassandra.cql3.ViewTest .ttlExpirationTest org.apache.cassandra.cql3.validation.entities .UFTest.testDropKeyspaceContainingFunctionDropsPreparedStatementsWit hDelayedValues org.apache.cassandra.cql3.validation.entities .UFTest.testEmptyString All five test failures are due to timeouts. === dtest: All passed! === upgrade: All passed! === novnode: All passed! === trunk: === testall: 3 failures org.apache.cassandra.config.DatabaseDescriptorRefTest .testDatabaseDescriptorRef org.apache.cassandra.config.DatabaseDescriptorRefTest .testDatabaseDescriptorRef-compression CASSANDRA-12677 for the two failures above. New failure org.apache.cassandra.dht.tokenallocator.ReplicationAwareTokenAllocatorTest .testNewClusterWithMurmur3Partitioner Failed due to timeout === dtest: All passed! === upgrade: All passed! === novnode: 8 failures All failures are in paging test, and are due to CASSANDRA-12666 which is Patch Available.
Re: Question on assert
Yes obviously we do not need to go in and replace them all at once. Some rough guidance/general consensus should be in place, because we are violating the standard usage: https://docs.oracle.com/javase/8/docs/technotes/guides/language/assert.html Do *not* use assertions for argument checking in public methods. Do *not* use assertions to do any work that your application requires for correct operation. There should be a rational as to why and when this is right. Otherwise changes like this might be considered bikeshedding. In any case I created https://issues.apache.org/jira/browse/CASSANDRA-12688 since I think we can all agree that can not run without them at the moment and we do not want to give someone an incentive to set this off which I feel the claim of 5% performance does. On Thu, Sep 22, 2016 at 7:29 AM, Benjamin Lerer wrote: > I fully agree. > > On Thu, Sep 22, 2016 at 11:57 AM, Dave Brosius > wrote: > > > As an aside, C* for some reason heavily uses asserts in unit tests, which > > adds to the "can't see the forest for the trees" problem. I see no reason > > for that. they should all be moved over to junit asserts. > > > > > > > > On 09/22/2016 03:52 AM, Benjamin Lerer wrote: > > > >> We can spend hours arguing about assert vs exceptions. I have seen it > >> happen in every company I worked for. > >> Overall, based on the patches I have reviewed, it seems to me that in > >> general people are using them only has internal safety checks. > >> Unfortunatly, the code change and we can miss things. > >> If anybody think that some SPECIFIC assertions should be replaced by > some > >> real checks, I think the best way to do it is to open a JIRA ticket to > >> raise the problem. > >> > >> > > >
Re: Guidelines for test coverage
Hi Nate, Good we have these guidelines. I have seen several parts of the code which were added without any tests over the years. We should at the minimum make it mandatory to add tests specially when adding major features to the code base. Thanks, Sankalp On Wed, Sep 21, 2016 at 3:27 PM, Dave Lester wrote: > A related question: > > I noticed that dtest is not part of the Apache project: > https://github.com/riptano/cassandra-dtest. Are there plans to contribute > this to the ASF? > > Dave > > > On Sep 21, 2016, at 3:17 PM, Nate McCall wrote: > > > > We have these already written down in from both the reviewing and > > development perspective: > > http://cassandra.apache.org/doc/latest/development/how_to_review.html > > http://cassandra.apache.org/doc/latest/development/testing.html > > > > My takeaway from recent in-person discussions the other week about this > > topic, is that it's really just a matter of making this less of a > > chore/sharing the burden of reviews when the whole dtest suite has to be > > run. > > > > Thanks for bringing this up, though. > > > > On Tue, Sep 20, 2016 at 6:42 AM, sankalp kohli > > wrote: > > > >> Hi, > >>I wanted to know if there are any guidelines for contributors to give > >> out unit and integration tests along with the patches. If not, we should > >> discuss and have them in place. > >> > >> I know we are making good progress with test coverage but we should add > >> guidelines around adding unit and integration tests with every patch. > >> Thoughts on this? > >> > >> Thanks, > >> Sankalp > >> > > > > > > > > -- > > - > > Nate McCall > > Wellington, NZ > > @zznate > > > > CTO > > Apache Cassandra Consulting > > http://www.thelastpickle.com > >
Thanks to Jonathan for his service as PMC Chair
Folks, Yesterday, I officially stepped into the role of PMC chair for Apache Cassandra. It is important to me that the first email I send in this capacity be a simple and sincere thank you for all the effort Jonathan Ellis has put into this position. Best regards, -Nate
[Discuss] Adding dtest to project
[Moved from PMC as there is nothing really private involved] DataStax has graciously offered to contribute cassandra-dtest [0] to the project. There were, however, two issues noted by Jonathan when he presented the offer to the PMC: 1. dtest mixes tests for many cassandra versions together in a single project. So having it live in the main cassandra repo, versioned by cassandra release, doesn't really make sense. Is Infra able to create a second repo for this, or is the "one project, one repo" mapping fixed? 2. DataStax did not require a CLA to contribute to dtest, so the non-DS contributors to dtest would need to be contacted for their permission to assign copyright to the ASF. Is the PMC willing to tackle this? In a brief discussion, it was deduced that #1 can be addressed by adding apache/cassandra-dtest to the ASF repo (the example of apache/aurora and apache/aurora-packaging was given to justify this). #2 will be harder as it will require tracking a few people people down to sign ASF CLAs. Before we really put effort into this, I wanted to open the discussion up about whether we are willing to take on the development of this in the project. Thoughts? -Nate [0] https://github.com/riptano/cassandra-dtest
