Refactored for CI - Githubissues

Cerfoglg commented 8 years ago

Warning: Massive amount of refactoring has been done

fixes #41 and implements #56

[ ] File: test/runTests.sh:L1-128
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Why do you have different for?
[ ] File: test/runTests.sh:L1-128
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Move configurations to configuration variables. This applies to all the code.
[ ] File: test/runTests.sh:L1-128
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Move this to a variable. This applies to all the code.
[ ] File: test/runTests.sh:L1-128
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Move these strings to variables. This applies to all the code.
[ ] File: test/spark/cassandraTest.py:L1-154
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Strings that are equal should go to configuration variables.
[ ] File: analysers/trials/throughput.py:L5-92
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Unused SparkFiles imported from pyspark
[ ] File: analysers/trials/throughput.py:L5-92
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Unused import yaml
[ ] File: test/spark/throughputTest.py:L1-75
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: test/spark/throughputTest.py:L1-75
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Unused RowFormat imported from pyspark_cassandra
[ ] File: test/spark/cutNInitialProcessesTest.py:L1-76
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: analysers/trials/databaseSize.py:L4-53
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Using type() instead of isinstance() for a typecheck.
[ ] File: analysers/experiments/ram.py:L5-73
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Unused variable 'trialID'
[ ] File: analysers/trials/databaseSize.py:L4-53
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: analysers/experiments/cpu.py:L15-165
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Unused variable 'trialID'
[ ] File: analysers/trials/databaseSize.py:L9-53
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: test/spark/computeExperimentsMetricsTest.py:L1-234
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: analysers/trials/databaseSize.py:L9-53
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: test/spark/cutNInitialProcessesTest.py:L1-76
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Unused RowFormat imported from pyspark_cassandra
[ ] File: test/spark/IOTest.py:L1-106
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: test/spark/cutNInitialProcessesTest.py:L1-76
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: test/spark/IOTest.py:L1-106
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Unused RowFormat imported from pyspark_cassandra
[ ] File: analysers/trials/cpu.py:L15-154
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: No exception type(s) specified
[ ] File: test/spark/computeExperimentCoreMetricsTest.py:L1-120
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Unused RowFormat imported from pyspark_cassandra
[ ] File: test/spark/computeExperimentCoreMetricsTest.py:L1-120
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: test/spark/cutNInitialProcessesTest.py:L1-76
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Unused RowFormat imported from pyspark_cassandra
[ ] File: test/spark/cutNInitialProcessesTest.py:L1-76
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: analysers/experiments/IO.py:L15-89
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: analysers/trials/IO.py:L9-87
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: test/spark/computeExperimentsMetricsTest.py:L1-234
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: analysers/trials/throughput.py:L11-91
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Unused SparkFiles imported from pyspark
[ ] File: analysers/trials/numberOfProcessInstances.py:L9-50
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Trailing whitespace
[ ] File: test/spark/computeModeTest.py:L1-81
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Issue found: Unused variable 'i'
[x] General Comment
<img border=0 src='https://avatars.githubusercontent.com/u/8819678?v=3' height=16 width=16'> To change:
trusty to precise in travis.yml
change wait to using a container to check for cassandra to be ready
move Dockerfile to test dir, change name to Dockerfile.test
maintain same file structure in the container
modeMin and modeMax should be a separate function from experimentMetrics
take a look at sortByKey when it's used, and whenever it should be separate
experimentID should be an argument
use external configuration for scripts for value such as cassandra keyspace
conf should only set app name
noOfActiveCores change to different name
in exp cpu put configuration for dest table
cpu cores from properties
Alluxio for cuts
benchflow.cql downloaded from Git
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> @Cerfoglg I see the following as missing:
noOfActiveCores change to different name. What it actually represents?
number of total cpu cores from container_properties
Alluxio for cuts
Add a more meaningful message to the test assertion. If it fails, write also the expected value.
Use more meaningful names for the test functions. E.g., TestOne is not meaningful, TestEmptyData is meaningful.
Fix reported error in code
Fix conflicts so that it can be merged. Currently it is not possible to merge it to the dev branch.
<img border=0 src='https://avatars.githubusercontent.com/u/8819678?v=3' height=16 width=16'> @VincenzoFerme It's all solved now, aside from alluxio enabling #63 , which I'll do in another pull, and some of these nitpicks the bot found (invisible trailing whitespaces)
<img border=0 src='https://avatars.githubusercontent.com/u/7163641?v=3' height=16 width=16'> Looks 👍 I disabled invisible trailing whitespace. For now I merge the pull request anyway, even though codeclimate reports many errors. We have a dedicated issue where I link to all the code improvements to perform: https://github.com/benchflow/analysers/issues/59

Cerfoglg commented 8 years ago

To change:

trusty to precise in travis.yml
change wait to using a container to check for cassandra to be ready
move Dockerfile to test dir, change name to Dockerfile.test
maintain same file structure in the container
modeMin and modeMax should be a separate function from experimentMetrics
take a look at sortByKey when it's used, and whenever it should be separate
experimentID should be an argument
use external configuration for scripts for value such as cassandra keyspace
conf should only set app name
noOfActiveCores change to different name
in exp cpu put configuration for dest table
cpu cores from properties
Alluxio for cuts
benchflow.cql downloaded from Git

VincenzoFerme commented 8 years ago

@Cerfoglg I see the following as missing:

noOfActiveCores change to different name. What it actually represents?
number of total cpu cores from container_properties
Alluxio for cuts
Add a more meaningful message to the test assertion. If it fails, write also the expected value.
Use more meaningful names for the test functions. E.g., TestOne is not meaningful, TestEmptyData is meaningful.
Fix reported error in code
Fix conflicts so that it can be merged. Currently it is not possible to merge it to the dev branch.

Cerfoglg commented 8 years ago

@VincenzoFerme It's all solved now, aside from alluxio enabling #63 , which I'll do in another pull, and some of these nitpicks the bot found (invisible trailing whitespaces)

VincenzoFerme commented 8 years ago

Looks 👍

I disabled invisible trailing whitespace.

For now I merge the pull request anyway, even though codeclimate reports many errors. We have a dedicated issue where I link to all the code improvements to perform: https://github.com/benchflow/analysers/issues/59

VincenzoFerme commented 8 years ago

:shipit:

benchflow / analysers

Refactored for CI #57