Hi,

I'd like to address some issues with S2Graph test code.

1. The tests do not cover all schema versions:
The latest version of S2Graph has four schema versions (v1 through v4).
Yet, most of the current test cases only cover one or two versions (usually
v2 or v3). Each test case should run against all versions that support the
feature being tested.

2. No option for different storages:
S2Graph has plans to support storages other than HBase (RocksDB, Redis, and
so on). But, test cases such as 'AsynchbaseStorageTest ' aren't necessary
for RocksDB. In this case, it would be more developer-friendly to provide
an option to run only the tests that concern a given storage. In other
words, if one is using S2Graph with RocksDB, she should be able to run the
test cases that covers common or RocksDB-related features only, and skip
the ones like 'AsynchbaseStorageTest'.

3. Mixed use of different testing styles:
S2Graph uses ScalaTest which supports several testing styles (
http://www.scalatest.org/user_guide/selecting_a_style). As is, multiple
styles are used in the test code, FunSuite and FlatSpec. I think it's
better to stick to one style.


My suggestion regarding 1 and 2 is to use tags (
http://www.scalatest.org/user_guide/tagging_your_tests).
This way, you can configure the test suite to run selective test cases with
a predefined SBT command.
For example, let's say all the test cases are tagged according to their
supported versions or storages, and Redis only supports schema version v4.
In 'build.sbt', you can define that a predefined command such as
'redis:test' should only run the tests that are tagged either 'v4' or
'Redis'.

Issue 3 is merely a matter of deciding witch style roll with and simply
rewriting the code.

Let me know what you think, and I'll open a JIRA ticket for this.

Regards,
Jo

Reply via email to