fix: isCometEnabled name conflict

[kazuyukitanimura]

Which issue does this PR close?

Rationale for this change

We have a name collision on isCometEnabled

CometSparkSessionExtensions
private[comet] def isCometEnabled(conf: SQLConf): Boolean = {

and

SQLTestUtilsBase
protected def isCometEnabled: Boolean = SparkSession.isCometEnabled

causing build issues depending on which one gets picked up

[ERROR] /workspace/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala:34: Boolean does not take parameters
[ERROR] /workspace/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala:42: Boolean does not take parameters
[ERROR] /workspace/spark/src/test/scala/org/apache/comet/CometSparkSessionExtensionsSuite.scala:47: Boolean does not take parameters

What changes are included in this PR?

Renamed one to isCometLoaded

How are these changes tested?

CI

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.49%. Comparing base (f09f8af) to head (667c19e).
Report is 103 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1569      +/-   ##
============================================
+ Coverage     56.12%   58.49%   +2.37%     
- Complexity      976      977       +1     
============================================
  Files           119      122       +3     
  Lines         11743    12231     +488     
  Branches       2251     2278      +27     
============================================
+ Hits           6591     7155     +564     
+ Misses         4012     3945      -67     
+ Partials       1140     1131       -9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[parthchandra]

TBH, I don't see how this conflict is occurring. CometSparkSessionExtensionSuite has no reference to SQLTestUtilsBase

[parthchandra]

Maybe use CometSparkSessionExtensions.isCometEnabled instead of renaming the method?

[comphead]

+1

[kazuyukitanimura]

Thanks, I thought about it but since both version of isCometEnabled would be still accessible, especially due to

  import CometSparkSessionExtensions._

, this may cause future issues for using isCometEnabled like adding new tests.
I am ok with CometSparkSessionExtensions.isCometEnabled but changing the name entirely is more future proof? WDTY @parthchandra @comphead
Also CometSparkSessionExtensions.isCometEnabled is checking NativeBase.isLoaded, so I think isCometLoaded makes sense

[parthchandra]

Thanks, I thought about it but since both version of isCometEnabled would be still accessible, especially due to

  import CometSparkSessionExtensions._

, this may cause future issues for using isCometEnabled like adding new tests. I am ok with CometSparkSessionExtensions.isCometEnabled but changing the name entirely is more future proof? WDTY @parthchandra @comphead Also CometSparkSessionExtensions.isCometEnabled is checking NativeBase.isLoaded, so I think isCometLoaded makes sense

It used to be CometSparkSessionExtensions.isCometEnabled and was changed here: badbd37

I'm okay with the new name though.

[kazuyukitanimura]

TBH, I don't see how this conflict is occurring. CometSparkSessionExtensionSuite has no reference to SQLTestUtilsBase

CometSparkSessionExtensionSuite -> CometTestBase -> SQLTestUtils -> SQLTestUtilsBase (in the diff)

[comphead]

as long as there is no end user changes I think we can go and merge the PR

[kazuyukitanimura]

Thanks merged @parthchandra @comphead