You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2021/04/09 12:24:14 UTC

[GitHub] [cassandra] jacek-lewandowski opened a new pull request #957: CASSANDRA-16575: Refactor unit test configurations

jacek-lewandowski opened a new pull request #957:
URL: https://github.com/apache/cassandra/pull/957


   Instead of predefined test, test-compression, test-cdc, test-system-keyspace-directory, have the ability to specify a properties file, which may include extra-java-opts property. That property contains extra JVM options to be added to a forked JVM for testing.
   
   This way, instead of:
   
   ant test-compression
   
   we have:
   ant -Dprops=test/conf/unit-with-sstable-compression.properties test
   
   We can easily create new configurations or modify existing without touching the build itself
   
   Also changes the compressor used in compression configuration from snappy to lz4


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra] michaelsembwever commented on a change in pull request #957: CASSANDRA-16575: Refactor unit test configurations

Posted by GitBox <gi...@apache.org>.
michaelsembwever commented on a change in pull request #957:
URL: https://github.com/apache/cassandra/pull/957#discussion_r615449359



##########
File path: test/conf/commitlog_compression_Zstd.yaml
##########
@@ -1,2 +0,0 @@
-commitlog_compression:
-    - class_name: ZstdCompressor

Review comment:
       this should be maintained. compression tests should enable both sstable and commitlog compression.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra] michaelsembwever commented on a change in pull request #957: CASSANDRA-16575: Refactor unit test configurations

Posted by GitBox <gi...@apache.org>.
michaelsembwever commented on a change in pull request #957:
URL: https://github.com/apache/cassandra/pull/957#discussion_r610713357



##########
File path: build.xml
##########
@@ -1444,75 +1447,6 @@
     </sequential>
   </macrodef>
 
-  <!-- Will not generate a junit report or fail on error since it is called in parallel for test-compression
-       That is taken care of by testparallel -->
-  <macrodef name="testlist-compression">
-    <attribute name="test.file.list" />
-    <attribute name="testlist.offset" />
-    <sequential>
-      <property name="compressed_yaml" value="${build.test.dir}/cassandra.compressed.yaml"/>
-      <concat destfile="${compressed_yaml}">
-          <fileset file="${test.conf}/cassandra.yaml"/>
-          <fileset file="${test.conf}/commitlog_compression.yaml"/>
-      </concat>
-      <testmacrohelper inputdir="${test.unit.src}" filelist="@{test.file.list}" poffset="@{testlist.offset}"
-                       exclude="**/*.java" timeout="${test.timeout}" testtag="compression">
-        <jvmarg value="-Dlegacy-sstable-root=${test.data}/legacy-sstables"/>
-        <jvmarg value="-Dinvalid-legacy-sstable-root=${test.data}/invalid-legacy-sstables"/>
-        <jvmarg value="-Dcassandra.test.compression=true"/>
-        <jvmarg value="-Dcassandra.ring_delay_ms=1000"/>
-        <jvmarg value="-Dcassandra.tolerate_sstable_size=true"/>
-        <jvmarg value="-Dcassandra.config=file:///${compressed_yaml}"/>
-        <jvmarg value="-Dcassandra.skip_sync=true" />
-        <jvmarg value="-Dcassandra.config.loader=org.apache.cassandra.OffsetAwareConfigurationLoader"/>
-      </testmacrohelper>
-    </sequential>
-  </macrodef>
-
-  <macrodef name="testlist-cdc">
-    <attribute name="test.file.list" />
-    <attribute name="testlist.offset" />
-    <sequential>
-      <property name="cdc_yaml" value="${build.test.dir}/cassandra.cdc.yaml"/>
-      <concat destfile="${cdc_yaml}">
-        <fileset file="${test.conf}/cassandra.yaml"/>
-        <fileset file="${test.conf}/cdc.yaml"/>

Review comment:
       is there any way of maintaining this feature?  it is cumbersome to keep all the different test cassandra.yaml's up to date… 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra] michaelsembwever closed pull request #957: CASSANDRA-16575: Refactor unit test configurations

Posted by GitBox <gi...@apache.org>.
michaelsembwever closed pull request #957:
URL: https://github.com/apache/cassandra/pull/957


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra] michaelsembwever commented on a change in pull request #957: CASSANDRA-16575: Refactor unit test configurations

Posted by GitBox <gi...@apache.org>.
michaelsembwever commented on a change in pull request #957:
URL: https://github.com/apache/cassandra/pull/957#discussion_r615449193



##########
File path: build.xml
##########
@@ -69,6 +69,8 @@
     <property name="test.driver.read_timeout_ms" value="12000"/>
     <property name="dist.dir" value="${build.dir}/dist"/>
     <property name="tmp.dir" value="${java.io.tmpdir}"/>
+    <property file="${props}"/>
+    <property name="extra-java-opts" value=""/>

Review comment:
       would it be possible to encapsulate these just related test targets?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra] michaelsembwever commented on a change in pull request #957: CASSANDRA-16575: Refactor unit test configurations

Posted by GitBox <gi...@apache.org>.
michaelsembwever commented on a change in pull request #957:
URL: https://github.com/apache/cassandra/pull/957#discussion_r610713357



##########
File path: build.xml
##########
@@ -1444,75 +1447,6 @@
     </sequential>
   </macrodef>
 
-  <!-- Will not generate a junit report or fail on error since it is called in parallel for test-compression
-       That is taken care of by testparallel -->
-  <macrodef name="testlist-compression">
-    <attribute name="test.file.list" />
-    <attribute name="testlist.offset" />
-    <sequential>
-      <property name="compressed_yaml" value="${build.test.dir}/cassandra.compressed.yaml"/>
-      <concat destfile="${compressed_yaml}">
-          <fileset file="${test.conf}/cassandra.yaml"/>
-          <fileset file="${test.conf}/commitlog_compression.yaml"/>
-      </concat>
-      <testmacrohelper inputdir="${test.unit.src}" filelist="@{test.file.list}" poffset="@{testlist.offset}"
-                       exclude="**/*.java" timeout="${test.timeout}" testtag="compression">
-        <jvmarg value="-Dlegacy-sstable-root=${test.data}/legacy-sstables"/>
-        <jvmarg value="-Dinvalid-legacy-sstable-root=${test.data}/invalid-legacy-sstables"/>
-        <jvmarg value="-Dcassandra.test.compression=true"/>
-        <jvmarg value="-Dcassandra.ring_delay_ms=1000"/>
-        <jvmarg value="-Dcassandra.tolerate_sstable_size=true"/>
-        <jvmarg value="-Dcassandra.config=file:///${compressed_yaml}"/>
-        <jvmarg value="-Dcassandra.skip_sync=true" />
-        <jvmarg value="-Dcassandra.config.loader=org.apache.cassandra.OffsetAwareConfigurationLoader"/>
-      </testmacrohelper>
-    </sequential>
-  </macrodef>
-
-  <macrodef name="testlist-cdc">
-    <attribute name="test.file.list" />
-    <attribute name="testlist.offset" />
-    <sequential>
-      <property name="cdc_yaml" value="${build.test.dir}/cassandra.cdc.yaml"/>
-      <concat destfile="${cdc_yaml}">
-        <fileset file="${test.conf}/cassandra.yaml"/>
-        <fileset file="${test.conf}/cdc.yaml"/>

Review comment:
       is there any way of maintaining this feature?  it is cumbersome (and error-prone) to keep all the different test cassandra.yaml's up to date… 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra] michaelsembwever commented on a change in pull request #957: CASSANDRA-16575: Refactor unit test configurations

Posted by GitBox <gi...@apache.org>.
michaelsembwever commented on a change in pull request #957:
URL: https://github.com/apache/cassandra/pull/957#discussion_r619515534



##########
File path: test/unit/org/apache/cassandra/SchemaLoader.java
##########
@@ -751,4 +747,26 @@ public static void cleanupSavedCaches()
     {
         ServerTestUtils.cleanupSavedCaches();
     }
+
+    private static CompressionParams compressionParams(int chunkLength)
+    {
+        String algo = System.getProperty("test.compression_algo", "lz4").toLowerCase();

Review comment:
       ```suggestion
           String algo = System.getProperty("cassandra.test.compression.algo", "lz4").toLowerCase();
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra] michaelsembwever commented on pull request #957: CASSANDRA-16575: Refactor unit test configurations

Posted by GitBox <gi...@apache.org>.
michaelsembwever commented on pull request #957:
URL: https://github.com/apache/cassandra/pull/957#issuecomment-828276723


   merged with https://github.com/apache/cassandra/commit/10a1d65eb09a93aee32948b46b4f1a0fbc2defe0


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


[GitHub] [cassandra] michaelsembwever commented on pull request #957: CASSANDRA-16575: Refactor unit test configurations

Posted by GitBox <gi...@apache.org>.
michaelsembwever commented on pull request #957:
URL: https://github.com/apache/cassandra/pull/957#issuecomment-822055833


   A few small comments added.
   
   Compatibility over branches is also a concern here. We currently have one [build script](https://github.com/apache/cassandra-builds/blob/trunk/build-scripts/cassandra-test.sh#L93-L104) that kicks off the ant command line for the different test targets, that runs regardless of the cassandra branch.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org