Add ability to ignore tests depending on spark shim version

[andygrove]

Signed-off-by: Andy Grove andygrove@nvidia.com

Adds the ability to ignore scala tests conditionally, depending on the version of Spark we are testing with.

Also closes #382

[tgravescs]

build

[andygrove]

Although it seems unnecessary to have this separate variable, it makes the output message more obvious:

Test Canceled: isValidTestForSparkVersion was false

[tgravescs]

so the SparkShimServiceProvider in each of the directories already has a VERSIONNAME string in it. It would be nice if we didn't have to duplicate it twice. Also you are missing the 300 databricks version here. I assume you added the class DatabricksShimVersion for it? we may need more information for databricks - runtime - we can always extend it later though as well.

[andygrove]

I wasn't sure how this would work with Databricks or what the version numbers would be. The case classes might give us more flexibility than strings and could perhaps use these instead of the string for version name?

I pushed another commit to demonstrate how I imagined this working for databricks.

[tgravescs]

we can use the case classes as the real version and then have it have a function toString, that is used in SparkShimServiceProvider for now. That way we don't have the version in 2 places.

I would like to add it to the databricks version as well. I don't want to have to fix it afterwards. Have it run the tests for it as it is now and the test is passing. They only changed this in 3.1 and so far 7.0 databricks runtime doesn't have that change. If the test ends up failing we can handle it separate but I would at least like to make an attempt here otherwise compilation will fail. The databricks version is just 3.0.0-databricks. It is not 7.0 which is the runtime version.

The downside to having different case classes is demonstrated here, where you would have to match on both. If we had a "vendor" field in SparkShimVersion you could conditionalize that. In this case your check (noticed extra _)
SparkShimVersion(major, minor, _, _) => major == 3 && minor == 0
would still work and cover databricks.
I can see reasons to do it both ways though. For instance this way would allow the Databricks version to have a separate runtime field in it as well. I'm fine with leaving it like this for now with different case classes, but please add the version to the 300db Shim layer as DatabricksShimVersion(3,0,0) and have the test run for it.

I think either way we will have to enhance the databricks version and probably this anyway to handle cases where the version is like 3.0.0.0 - but there is a separate jira for that.

[andygrove]

Thanks @tgravescs I hadn't realized that the databricks shim was already merged in. I have made those updates although this change has turned out larger than I originally thought. Let me know what you think. I'm happy to take a different approach if this is too disruptive.

[tgravescs]

overall I think this is fine, few minor nits. We can enhance it as we need.

[tgravescs]

build

[andygrove]

build

[andygrove]

@tgravescs I addressed those nits