Skip to content

Conversation

@tchow-zlai
Copy link
Collaborator

@tchow-zlai tchow-zlai commented Aug 17, 2025

Summary

Checklist

  • Added Unit Tests
  • Covered by existing CI
  • Integration tested
  • Documentation update

Summary by CodeRabbit

  • Refactor
    • Removed a deprecated configuration type from the public planner API.
  • Tests
    • Consolidated test packages under primary namespaces and updated imports across modules; no test logic changes.
  • Chores
    • Simplified and cleaned up imports in test suites to reduce redundancy and align with new package structure.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 17, 2025

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Tests across aggregator, api, and spark modules were moved from *.test packages into their parent packages, with corresponding import path updates and minor import simplifications. One API change removes the Mode case class from ConfPlanner.scala. No test logic or runtime control-flow changes are introduced.

Changes

Cohort / File(s) Summary of Changes
Aggregator tests package shift
aggregator/src/test/scala/ai/chronon/aggregator/*(ApproxDistinctTest.scala, ApproxPercentilesTest.scala, EditDistanceTest.scala, FrequentItemsTest.scala, MinHeapTest.scala, MomentTest.scala, NaiveAggregator.scala, RowAggregatorTest.scala, SawtoothAggregatorTest.scala, SawtoothOnlineAggregatorTest.scala, TwoStackLiteAggregatorTest.scala, UniqueTopKTest.scala, VarianceTest.scala)
Package changed: ai.chronon.aggregator.test → ai.chronon.aggregator; adjusted imports referencing test-qualified symbols (e.g., sawtoothAggregate).
Aggregator DataGen import tweak
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala
Package changed to ai.chronon.aggregator; CStream import updated to match new package.
API planner public type removal
api/src/main/scala/ai/chronon/api/planner/ConfPlanner.scala
Removed case class Mode(name: String, nodes: Seq[Node], cron: String).
API tests package shift
api/src/test/scala/ai/chronon/api/*(CollectionExtensionsTest.scala, DataPointerTest.scala, DataTypeConversionTest.scala, DateMacroSpec.scala, ExtensionsTest.scala, RelevantLeftForJoinPartSpec.scala, TileSeriesSerializationTest.scala, TilingUtilSpec.scala, TimeExpressionSpec.scala)
Package changed: ai.chronon.api.test → ai.chronon.api; simplified/removed redundant imports (e.g., switched to relative or wildcard).
API planner tests package shift
api/src/test/scala/ai/chronon/api/planner/*(ConfPlannerTest.scala, GroupByPlannerTest.scala, LocalRunnerTest.scala, MetaDataUtilsTest.scala, MonolithJoinPlannerTest.scala, StagingQueryPlannerTest.scala, TableDependenciesTest.scala)
Package changed: ai.chronon.api.test.planner → ai.chronon.api.planner; replaced specific imports with ai.chronon.api._ and removed now-unnecessary planner imports.
Spark tests import migration (aggregator symbols)
spark/src/test/scala/ai/chronon/spark/test/*(DataFrameGen.scala, LocalTableExporterTest.scala, MetadataExporterTest.scala, MigrationCompareTest.scala, StagingQueryTest.scala, StatsComputeTest.scala, TestUtils.scala)
Imports updated from ai.chronon.aggregator.test.(Column, CStream, RowsWithSchema, NaiveAggregator, etc.) → ai.chronon.aggregator.(...). No logic changes.
Spark analyzer/batch imports
spark/src/test/scala/ai/chronon/spark/test/analyzer/AnalyzerTest.scala, .../batch/*(EvalTest.scala, MergeJobAnalyzeReuseTest.scala, MergeJobVersioningTest.scala, ModularJoinTest.scala, ShortNamesTest.scala)
Column import path switched to ai.chronon.aggregator.Column.
Spark bootstrap/fetcher imports
spark/src/test/scala/ai/chronon/spark/test/bootstrap/BootstrapUtils.scala, .../fetcher/FetcherTestUtil.scala
Column import path switched to ai.chronon.aggregator.Column.
Spark groupby imports
spark/src/test/scala/ai/chronon/spark/test/groupby/*(GroupByTest.scala, GroupByUploadTest.scala)
Imports moved from ai.chronon.aggregator.test → ai.chronon.aggregator (Column, CStream, NaiveAggregator).
Spark join imports
spark/src/test/scala/ai/chronon/spark/test/join/*(BaseJoinTest.scala, DifferentPartitionColumnsTest.scala, EntitiesEntitiesTest.scala, EventsEntitiesSnapshotTest.scala, EventsEventsSnapshotTest.scala, EventsEventsTemporal*.scala, HeterogeneousPartitionColumnsTest.scala, JoinUtilsTest.scala, KeyMappingOverlappingFieldsTest.scala, NoAggTest.scala, NoHistoricalBackfillTest.scala, SawtoothUdfPerformanceTest.scala, SelectedJoinPartsTest.scala, SkipBloomFilterJoinBackfillTest.scala, StructJoinTest.scala, UnionJoinTest.scala)
Column import path switched to ai.chronon.aggregator.Column; several files broaden ai.chronon.api imports to ai.chronon.api._; removed unused FiveMinuteResolution in one test.
Spark streaming imports
spark/src/test/scala/ai/chronon/spark/test/streaming/*(AvroTest.scala, MutationsTest.scala, StreamingTest.scala)
Column import path switched to ai.chronon.aggregator.Column.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • varant-zlai
  • piyush-zlai
  • nikhil-zlai
  • chewy-zlai

Poem

Packages packed their bags, took flight,
From .test to base, they found the light.
Imports trimmed, the clutter gone,
A tidy path to build upon.
Mode waved bye in planner’s dome—
Tests still run, all systems home. ✨

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch tchow/repackage

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@tchow-zlai tchow-zlai changed the title wip wip Repackage unit tests Aug 17, 2025
@tchow-zlai tchow-zlai marked this pull request as draft August 17, 2025 16:58
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🔭 Outside diff range comments (3)
spark/src/test/scala/ai/chronon/spark/test/join/JoinUtilsTest.scala (2)

72-75: These tests don’t assert — convert comparisons into assertions.

Currently builds an Array[Boolean] and discards it.

-    expected.zip(actual).map { case (e, a) =>
-      e == a
-    }
+    expected.zip(actual).foreach { case (e, a) =>
+      assertEquals(e, a)
+    }

102-105: Same issue here — add assertions.

-    expected.zip(actual).map { case (e, a) =>
-      e == a
-    }
+    expected.zip(actual).foreach { case (e, a) =>
+      assertEquals(e, a)
+    }
spark/src/test/scala/ai/chronon/spark/test/join/SawtoothUdfPerformanceTest.scala (1)

203-211: Fix Spark DataType pattern matching (compile-time issue)

Match on singleton objects, not (non-existent) types. The current cases won’t compile reliably.

Apply:

   private def mapSparkTypeToChronon(dataType: types.DataType): DataType = {
     dataType match {
-      case _: types.IntegerType => IntType
-      case _: types.LongType    => LongType
-      case _: types.DoubleType  => DoubleType
-      case _: types.StringType  => StringType
-      case _                    => throw new IllegalArgumentException(s"Unsupported type: $dataType")
+      case types.IntegerType => IntType
+      case types.LongType    => LongType
+      case types.DoubleType  => DoubleType
+      case types.StringType  => StringType
+      case _                 => throw new IllegalArgumentException(s"Unsupported type: $dataType")
     }
   }
♻️ Duplicate comments (2)
spark/src/test/scala/ai/chronon/spark/test/groupby/GroupByTest.scala (1)

19-19: Updated imports from aggregator: OK; same cross-module note.

Looks good; ensure spark:test depends on aggregator:test (see prior verification comment).

spark/src/test/scala/ai/chronon/spark/test/join/KeyMappingOverlappingFieldsTest.scala (1)

19-19: Same cross-module test-jar concern as above

Ensure spark tests can resolve aggregator’s Column from aggregator’s test scope (publish/consume test-jar or Test->Test dep).

🧹 Nitpick comments (25)
spark/src/test/scala/ai/chronon/spark/test/join/StructJoinTest.scala (1)

21-21: Avoid wildcard import; import only what’s needed.

Reduces namespace pollution and mitigates potential clashes (e.g., with Spark types), while keeping the explicit api qualifier for DataTypes.

-import ai.chronon.api._
+import ai.chronon.api.{Accuracy, Builders, Operation, TimeUnit, Window}
spark/src/test/scala/ai/chronon/spark/test/join/NoHistoricalBackfillTest.scala (1)

19-19: Avoid future clash with Spark SQL Column

If org.apache.spark.sql.Column is imported in this scope later, this import will conflict. Consider aliasing (e.g., import ai.chronon.aggregator.{Column => AggColumn}) or fully-qualifying at call sites.

spark/src/test/scala/ai/chronon/spark/test/streaming/MutationsTest.scala (1)

23-29: Remove duplicate SparkSessionBuilder import

Minor cleanup; keep a single import.

 import ai.chronon.spark.submission.SparkSessionBuilder
@@
-import ai.chronon.spark.submission.SparkSessionBuilder
spark/src/test/scala/ai/chronon/spark/test/join/JoinUtilsTest.scala (1)

248-248: Remove debug output to keep test logs clean.

-    view.show()
spark/src/test/scala/ai/chronon/spark/test/join/SawtoothUdfPerformanceTest.scala (3)

146-151: Avoid hard-coded output index; derive from left schema

More robust if the left schema changes.

Apply:

-    // Function to extract the LAST_K items from a result
-    def extractLastKItems(row: CGenericRow): Array[_] = {
-      // Assuming LAST_K result is in column 3 (after user_id, query, ts)
-      row.get(3).asInstanceOf[Array[_]]
-    }
+    // Function to extract the LAST_K items from a result
+    // Index is left schema length (user_id, query, ts) before agg outputs.
+    val lastKColIndex: Int = leftSchema.length
+    def extractLastKItems(row: CGenericRow): Array[_] = {
+      row.get(lastKColIndex).asInstanceOf[Array[_]]
+    }

171-171: ScalaTest matcher: add parentheses with <=

Prevents parsing issues; preferred form in ScalaTest docs.

Apply:

-      sawtoothItems.size should be <= k
+      sawtoothItems.size should be <= (k)

39-39: Comment vs. cardinality mismatch for user_id

Comment says 10 user IDs, but cardinality is 1. Align for clarity/reproducibility.

Apply:

-      Column("user_id", IntType, 1), // 10 different user IDs
+      Column("user_id", IntType, 10), // 10 different user IDs
-      Column("user_id", IntType, 1), // Same user IDs as right
+      Column("user_id", IntType, 10), // Same user IDs as right

Also applies to: 57-57

api/src/test/scala/ai/chronon/api/planner/TableDependenciesTest.scala (2)

146-146: Nit: fix test description typo.

Add space in “shiftwhen”.

-  it should "prioritize partitionLag over shiftwhen both are provided" in {
+  it should "prioritize partitionLag over shift when both are provided" in {

156-159: Nit: inline comment contradicts assertions.

Comment says shift wins; assertions expect partitionLag. Fix the comment.

-    // shift should take precedence over partitionLag
+    // partitionLag should take precedence over shift
spark/src/test/scala/ai/chronon/spark/test/join/EventsEventsSnapshotTest.scala (1)

19-21: Imports look good; minor consistency nit

Mixing api._ with api-prefixed qualifiers is fine; consider using a single style for clarity (either rely on api._ and drop api. prefixes, or drop api._ and keep api. prefixes).

api/src/test/scala/ai/chronon/api/planner/MonolithJoinPlannerTest.scala (1)

54-58: Guard against missing RUNFILES_DIR in CI

If RUNFILES_DIR is unset, Paths.get(null, ...) will NPE. Consider a defensive guard to skip/cancel.

Example:

-    val runfilesDir = System.getenv("RUNFILES_DIR")
+    val runfilesDir = System.getenv("RUNFILES_DIR")
+    assume(runfilesDir != null && runfilesDir.nonEmpty, "RUNFILES_DIR not set")
api/src/test/scala/ai/chronon/api/planner/MetaDataUtilsTest.scala (1)

8-8: Prefer scala.jdk.CollectionConverters over JavaConverters

JavaConverters is deprecated in 2.13+. Align with other tests for consistency.

-import scala.collection.JavaConverters._
+import scala.jdk.CollectionConverters._
api/src/test/scala/ai/chronon/api/TileSeriesSerializationTest.scala (1)

16-21: Potential compile issue: use isInfinity/Java Double checks instead of isInfinite

In Scala, Double has isInfinity; isInfinite may not exist depending on version. Safer: use java.lang.Double static checks.

-    val percentileDrifts: Seq[JDouble] = Seq(0.1, null, Double.PositiveInfinity, Double.NaN, 0.5)
-      .map(v =>
-        if (v == null || (v != null && (v.asInstanceOf[Double].isInfinite || v.asInstanceOf[Double].isNaN)))
-          Constants.magicNullDouble
-        else
-          v.asInstanceOf[JDouble])
+    val percentileDrifts: Seq[JDouble] =
+      Seq[JDouble](0.1, null, Double.PositiveInfinity, Double.NaN, 0.5).map {
+        case null => Constants.magicNullDouble
+        case d if JDouble.isInfinite(d) || JDouble.isNaN(d) => Constants.magicNullDouble
+        case d => d
+      }
api/src/test/scala/ai/chronon/api/planner/GroupByPlannerTest.scala (2)

4-4: Importing companion method is optional

Since buildGroupBy is in the companion object, you can call GroupByPlannerTest.buildGroupBy without the import. Current form is fine.


86-90: Guard env-dependent canary tests

Avoid NPEs if RUNFILES_DIR is missing in some runners.

-    val runfilesDir = System.getenv("RUNFILES_DIR")
+    val runfilesDir = System.getenv("RUNFILES_DIR")
+    assume(runfilesDir != null && runfilesDir.nonEmpty, "RUNFILES_DIR not set")
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)

19-19: Prefer fully qualified import to avoid ambiguity (nit).

Being explicit helps if another CStream exists in the package.

Apply:

-import CStream._
+import ai.chronon.aggregator.CStream._
spark/src/test/scala/ai/chronon/spark/test/join/EventsEntitiesSnapshotTest.scala (2)

20-24: Standardize API type imports; drop redundant qualifier

Mixing api.StringType and StringType is inconsistent. Prefer unqualified types and remove the single-name import.

-import ai.chronon.api
 import ai.chronon.api.Extensions._
 import ai.chronon.api.ScalaJavaConversions._
 import ai.chronon.api._
@@
-      Column("user_name", api.StringType, 10),
+      Column("user_name", StringType, 10),
@@
-      Column("user_name", api.StringType, 10),
+      Column("user_name", StringType, 10),
@@
-      Column("user_name", api.StringType, 10),
-      Column("user", api.StringType, 10)
+      Column("user_name", StringType, 10),
+      Column("user", StringType, 10)

Also applies to: 33-37, 39-44, 92-94


73-77: Remove duplicate temp function registration

temp_replace_right_c is declared twice consecutively.

           "create temporary function temp_replace_right_b as 'org.apache.hadoop.hive.ql.udf.UDFRegExpReplace'",
           "create temporary function temp_replace_right_c as 'org.apache.hadoop.hive.ql.udf.UDFRegExpReplace'",
-          "create temporary function temp_replace_right_c as 'org.apache.hadoop.hive.ql.udf.UDFRegExpReplace'"
+          // removed duplicate temp_replace_right_c
aggregator/src/test/scala/ai/chronon/aggregator/SawtoothOnlineAggregatorTest.scala (1)

19-19: Prefer fully-qualified import to avoid ambiguity.
Use explicit package path for clarity.

-import SawtoothAggregatorTest.sawtoothAggregate
+import ai.chronon.aggregator.SawtoothAggregatorTest.sawtoothAggregate
aggregator/src/test/scala/ai/chronon/aggregator/TwoStackLiteAggregatorTest.scala (1)

20-20: Optional: use fully-qualified import for clarity.

Avoids ambiguity if similarly named symbols appear in scope.

Apply:

-import SawtoothAggregatorTest.sawtoothAggregate
+import ai.chronon.aggregator.SawtoothAggregatorTest.sawtoothAggregate
aggregator/src/test/scala/ai/chronon/aggregator/SawtoothAggregatorTest.scala (1)

20-20: Nit: importing from the companion object in the same file is unnecessary.

Prefer qualifying the call to avoid confusion between class and companion.

Apply:

-import SawtoothAggregatorTest.sawtoothAggregate

Then update the usage (outside this range) to:

val sawtoothIrs = SawtoothAggregatorTest.sawtoothAggregate(events, queries, aggregations, schema)
spark/src/test/scala/ai/chronon/spark/test/bootstrap/BootstrapUtils.scala (1)

20-21: Redundant package import and mixed qualification; unify for clarity.

You import both ai.chronon.api (for the api. prefix) and ai.chronon.api._ (wildcard), then mix LongType vs api.LongType. Pick one style. Minimal cleanup below drops the package alias and uses the wildcard consistently.

-import ai.chronon.api
 import ai.chronon.api._
@@
-      Column("user", api.LongType, 100),
-      Column("request_id", api.StringType, 100)
+      Column("user", LongType, 100),
+      Column("request_id", StringType, 100)

Also applies to: 63-65

spark/src/test/scala/ai/chronon/spark/test/StagingQueryTest.scala (3)

51-52: Gate .show() behind debug to reduce noisy logs.

-    logger.info("Generated staging query data:")
-    df.show()
+    if (logger.isDebugEnabled) {
+      logger.debug("Generated staging query data:")
+      df.show(20, false)
+    }

164-167: Avoid unconditional .show() in tests.

-    expectedUpdated.show()
+    if (logger.isDebugEnabled) expectedUpdated.show(20, false)
@@
-    computedUpdated.show()
+    if (logger.isDebugEnabled) computedUpdated.show(20, false)

352-352: Reuse computed diffCount to avoid recomputation.

-    assertEquals(0, diff.count())
+    assertEquals(0, diffCount)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8c587f4 and b3423ab.

📒 Files selected for processing (68)
  • aggregator/src/test/scala/ai/chronon/aggregator/ApproxDistinctTest.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/ApproxPercentilesTest.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/EditDistanceTest.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/FrequentItemsTest.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/MinHeapTest.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/MomentTest.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/NaiveAggregator.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/RowAggregatorTest.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/SawtoothAggregatorTest.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/SawtoothOnlineAggregatorTest.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/TwoStackLiteAggregatorTest.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/UniqueTopKTest.scala (1 hunks)
  • aggregator/src/test/scala/ai/chronon/aggregator/VarianceTest.scala (1 hunks)
  • api/src/main/scala/ai/chronon/api/planner/ConfPlanner.scala (0 hunks)
  • api/src/test/scala/ai/chronon/api/CollectionExtensionsTest.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/DataPointerTest.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/DataTypeConversionTest.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/DateMacroSpec.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/ExtensionsTest.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/RelevantLeftForJoinPartSpec.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/TileSeriesSerializationTest.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/TilingUtilSpec.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/TimeExpressionSpec.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/planner/ConfPlannerTest.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/planner/GroupByPlannerTest.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/planner/LocalRunnerTest.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/planner/MetaDataUtilsTest.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/planner/MonolithJoinPlannerTest.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/planner/StagingQueryPlannerTest.scala (1 hunks)
  • api/src/test/scala/ai/chronon/api/planner/TableDependenciesTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/DataFrameGen.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/LocalTableExporterTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/MetadataExporterTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/MigrationCompareTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/StagingQueryTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/StatsComputeTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/TestUtils.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/analyzer/AnalyzerTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/batch/EvalTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/batch/MergeJobAnalyzeReuseTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/batch/MergeJobVersioningTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/batch/ModularJoinTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/batch/ShortNamesTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/bootstrap/BootstrapUtils.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/fetcher/FetcherTestUtil.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/groupby/GroupByTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/groupby/GroupByUploadTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/BaseJoinTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/DifferentPartitionColumnsTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/EntitiesEntitiesTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/EventsEntitiesSnapshotTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/EventsEventsSnapshotTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/EventsEventsTemporalTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/EventsEventsTemporalWithGBDerivation.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/HeterogeneousPartitionColumnsTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/JoinUtilsTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/KeyMappingOverlappingFieldsTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/NoAggTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/NoHistoricalBackfillTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/SawtoothUdfPerformanceTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/SelectedJoinPartsTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/SkipBloomFilterJoinBackfillTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/StructJoinTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/join/UnionJoinTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/streaming/AvroTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/streaming/MutationsTest.scala (1 hunks)
  • spark/src/test/scala/ai/chronon/spark/test/streaming/StreamingTest.scala (1 hunks)
💤 Files with no reviewable changes (1)
  • api/src/main/scala/ai/chronon/api/planner/ConfPlanner.scala
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-12-03T04:04:33.809Z
Learnt from: nikhil-zlai
PR: zipline-ai/chronon#70
File: service/src/main/java/ai/chronon/service/ApiProvider.java:6-6
Timestamp: 2024-12-03T04:04:33.809Z
Learning: The import `scala.util.ScalaVersionSpecificCollectionsConverter` in `service/src/main/java/ai/chronon/service/ApiProvider.java` is correct and should not be flagged in future reviews.

Applied to files:

  • spark/src/test/scala/ai/chronon/spark/test/join/EventsEventsTemporalWithGBDerivation.scala
  • api/src/test/scala/ai/chronon/api/DataTypeConversionTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/batch/EvalTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/TestUtils.scala
  • spark/src/test/scala/ai/chronon/spark/test/MigrationCompareTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/join/HeterogeneousPartitionColumnsTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/LocalTableExporterTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/join/SelectedJoinPartsTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/join/EventsEventsTemporalTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/analyzer/AnalyzerTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/DataFrameGen.scala
  • spark/src/test/scala/ai/chronon/spark/test/streaming/AvroTest.scala
  • api/src/test/scala/ai/chronon/api/CollectionExtensionsTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/join/BaseJoinTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/join/JoinUtilsTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/batch/MergeJobAnalyzeReuseTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/join/DifferentPartitionColumnsTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/streaming/MutationsTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/batch/MergeJobVersioningTest.scala
  • spark/src/test/scala/ai/chronon/spark/test/join/EntitiesEntitiesTest.scala
  • api/src/test/scala/ai/chronon/api/TileSeriesSerializationTest.scala
  • api/src/test/scala/ai/chronon/api/ExtensionsTest.scala
📚 Learning: 2024-11-03T14:51:40.825Z
Learnt from: nikhil-zlai
PR: zipline-ai/chronon#50
File: spark/src/main/scala/ai/chronon/spark/stats/drift/SummaryUploader.scala:19-47
Timestamp: 2024-11-03T14:51:40.825Z
Learning: In Scala, the `grouped` method on collections returns an iterator, allowing for efficient batch processing without accumulating all records in memory.

Applied to files:

  • spark/src/test/scala/ai/chronon/spark/test/groupby/GroupByUploadTest.scala
📚 Learning: 2024-10-07T15:09:51.567Z
Learnt from: piyush-zlai
PR: zipline-ai/chronon#33
File: cloud_aws/src/test/scala/ai/chronon/integrations/aws/DynamoDBKVStoreTest.scala:175-175
Timestamp: 2024-10-07T15:09:51.567Z
Learning: Hardcoding future timestamps in tests within `DynamoDBKVStoreTest.scala` is acceptable when data is generated and queried within the same time range, ensuring the tests remain valid over time.

Applied to files:

  • api/src/test/scala/ai/chronon/api/TimeExpressionSpec.scala
🧬 Code Graph Analysis (47)
api/src/test/scala/ai/chronon/api/DateMacroSpec.scala (1)
api/src/main/scala/ai/chronon/api/ParametricMacro.scala (1)
  • api (66-66)
spark/src/test/scala/ai/chronon/spark/test/join/EventsEventsTemporalWithGBDerivation.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
api/src/test/scala/ai/chronon/api/DataTypeConversionTest.scala (1)
api/src/main/scala/ai/chronon/api/ParametricMacro.scala (1)
  • api (66-66)
spark/src/test/scala/ai/chronon/spark/test/batch/ShortNamesTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/batch/EvalTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/TestUtils.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/KeyMappingOverlappingFieldsTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/MigrationCompareTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/HeterogeneousPartitionColumnsTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/LocalTableExporterTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/NoAggTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/bootstrap/BootstrapUtils.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/SelectedJoinPartsTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/EventsEventsTemporalTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/analyzer/AnalyzerTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/groupby/GroupByUploadTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/DataFrameGen.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (4)
  • CStream (28-86)
  • CStream (88-172)
  • Column (174-204)
  • RowsWithSchema (205-205)
spark/src/test/scala/ai/chronon/spark/test/streaming/AvroTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
api/src/test/scala/ai/chronon/api/CollectionExtensionsTest.scala (1)
api/src/main/scala/ai/chronon/api/CollectionExtensions.scala (1)
  • CollectionExtensions (5-90)
spark/src/test/scala/ai/chronon/spark/test/join/BaseJoinTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/StatsComputeTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/JoinUtilsTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/NoHistoricalBackfillTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/streaming/StreamingTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/UnionJoinTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/batch/MergeJobAnalyzeReuseTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/fetcher/FetcherTestUtil.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/DifferentPartitionColumnsTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
aggregator/src/test/scala/ai/chronon/aggregator/SawtoothOnlineAggregatorTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/SawtoothAggregatorTest.scala (3)
  • SawtoothAggregatorTest (49-178)
  • SawtoothAggregatorTest (180-239)
  • sawtoothAggregate (184-238)
spark/src/test/scala/ai/chronon/spark/test/streaming/MutationsTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/MetadataExporterTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
api/src/test/scala/ai/chronon/api/TimeExpressionSpec.scala (1)
api/src/main/scala/ai/chronon/api/ColumnExpression.scala (1)
  • getTimeExpression (24-36)
spark/src/test/scala/ai/chronon/spark/test/join/SkipBloomFilterJoinBackfillTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
api/src/test/scala/ai/chronon/api/planner/StagingQueryPlannerTest.scala (1)
api/src/main/scala/ai/chronon/api/Builders.scala (2)
  • Builders (27-376)
  • MetaData (264-320)
spark/src/test/scala/ai/chronon/spark/test/StagingQueryTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/StructJoinTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/batch/MergeJobVersioningTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/EntitiesEntitiesTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
aggregator/src/test/scala/ai/chronon/aggregator/TwoStackLiteAggregatorTest.scala (2)
aggregator/src/main/scala/ai/chronon/aggregator/base/SimpleAggregators.scala (1)
  • TopK (645-646)
aggregator/src/test/scala/ai/chronon/aggregator/SawtoothAggregatorTest.scala (3)
  • SawtoothAggregatorTest (49-178)
  • SawtoothAggregatorTest (180-239)
  • sawtoothAggregate (184-238)
spark/src/test/scala/ai/chronon/spark/test/batch/ModularJoinTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
api/src/test/scala/ai/chronon/api/TileSeriesSerializationTest.scala (1)
api/src/main/scala/ai/chronon/api/ScalaJavaConversions.scala (1)
  • ScalaJavaConversions (6-97)
spark/src/test/scala/ai/chronon/spark/test/join/EventsEntitiesSnapshotTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/EventsEventsSnapshotTest.scala (1)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
spark/src/test/scala/ai/chronon/spark/test/join/SawtoothUdfPerformanceTest.scala (5)
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)
  • Column (174-204)
aggregator/src/test/scala/ai/chronon/aggregator/NaiveAggregator.scala (1)
  • NaiveAggregator (24-55)
aggregator/src/test/scala/ai/chronon/aggregator/SawtoothAggregatorTest.scala (1)
  • Timer (34-47)
aggregator/src/test/scala/ai/chronon/aggregator/RowAggregatorTest.scala (2)
  • TestRow (27-45)
  • TestRow (47-49)
spark/src/test/scala/ai/chronon/spark/test/join/BaseJoinTest.scala (2)
  • TestRow (28-28)
  • TestRow (30-37)
api/src/test/scala/ai/chronon/api/planner/MonolithJoinPlannerTest.scala (1)
api/src/main/scala/ai/chronon/api/Builders.scala (2)
  • Builders (27-376)
  • MetaData (264-320)
api/src/test/scala/ai/chronon/api/planner/GroupByPlannerTest.scala (3)
api/src/main/scala/ai/chronon/api/ParametricMacro.scala (1)
  • api (66-66)
api/src/main/scala/ai/chronon/api/Extensions.scala (3)
  • GroupByOps (501-732)
  • MetadataOps (157-222)
  • WindowUtils (90-155)
spark/src/test/scala/ai/chronon/spark/test/bootstrap/BootstrapUtils.scala (1)
  • buildGroupBy (29-59)
api/src/test/scala/ai/chronon/api/ExtensionsTest.scala (1)
api/src/main/scala/ai/chronon/api/ScalaJavaConversions.scala (1)
  • ScalaJavaConversions (6-97)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
  • GitHub Check: join_tests
  • GitHub Check: streaming_tests
  • GitHub Check: analyzer_tests
  • GitHub Check: groupby_tests
  • GitHub Check: online_tests
  • GitHub Check: batch_tests
  • GitHub Check: service_tests
  • GitHub Check: cloud_aws_tests
  • GitHub Check: spark_tests
  • GitHub Check: cloud_gcp_tests
  • GitHub Check: api_tests
  • GitHub Check: fetcher_tests
  • GitHub Check: service_commons_tests
  • GitHub Check: flink_tests
  • GitHub Check: aggregator_tests
  • GitHub Check: python_tests
  • GitHub Check: scala_compile_fmt_fix
  • GitHub Check: enforce_triggered_workflows
🔇 Additional comments (68)
spark/src/test/scala/ai/chronon/spark/test/join/EntitiesEntitiesTest.scala (1)

19-19: Import switch to ai.chronon.aggregator.Column: ensure test-jar is exposed to Spark tests

Column now lives only in tests (aggregator/src/test/scala/.../DataGen.scala); there’s no case class Column under aggregator/src/main. Before merging, confirm in your SBT setup that:

  • aggregator publishes its test artifacts (Test / publishArtifact := true or testsJars := true)
  • Spark’s project adds a dependency on the aggregator test-jar (e.g. % "test->test")

Otherwise the import in spark/src/test/scala/.../EntitiesEntitiesTest.scala (and all other Spark tests) will fail.

spark/src/test/scala/ai/chronon/spark/test/join/StructJoinTest.scala (1)

19-19: Import path realignment for Column looks correct.

Switching to ai.chronon.aggregator.Column aligns with the test helpers’ relocation. No functional impact expected.

spark/src/test/scala/ai/chronon/spark/test/join/UnionJoinTest.scala (2)

3-3: LGTM: switched to public Column import.

Matches the repo-wide refactor; no behavior change.


3-3: No import collisions detected
All files importing ai.chronon.aggregator.Column were checked; none also import org.apache.spark.sql.Column, and there are no lingering ai.chronon.aggregator.test.Column imports. Code is safe as-is.

spark/src/test/scala/ai/chronon/spark/test/streaming/StreamingTest.scala (1)

19-19: Confirm test-jar visibility & Column signature

  • Verify the Spark test module declares a test-scope dependency on the aggregator test-jar so ai.chronon.aggregator.Column (from DataGen.scala) is on its classpath.
  • Ensure the Column case-class constructor parameters (name: String, type: DataType, cardinality: Int, …) align with how it’s instantiated in StreamingTest.scala.
spark/src/test/scala/ai/chronon/spark/test/streaming/MutationsTest.scala (1)

19-19: Old Column imports fully removed; new import is correct

No occurrences of ai.chronon.aggregator.test.Column; the new
ai.chronon.aggregator.Column import aligns with the Column case class in:

  • aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala:174

Ready to merge.

spark/src/test/scala/ai/chronon/spark/test/join/EventsEventsTemporalWithGBDerivation.scala (1)

3-3: No legacy Column imports found; all references updated
Zero occurrences of ai.chronon.aggregator.test.Column and 34 instances of ai.chronon.aggregator.Column. LGTM—approving the import migration.

spark/src/test/scala/ai/chronon/spark/test/MigrationCompareTest.scala (1)

19-19: Import migration LGTM

Switch to ai.chronon.aggregator.Column is consistent with the test-package flattening and avoids Spark’s Column conflicts here.

spark/src/test/scala/ai/chronon/spark/test/batch/EvalTest.scala (1)

3-3: Import path switched to ai.chronon.aggregator.Column — LGTM

Matches the package consolidation; no symbol conflict with org.apache.spark.sql.Column in this file.

spark/src/test/scala/ai/chronon/spark/test/join/DifferentPartitionColumnsTest.scala (1)

19-19: Confirm test->test SBT wiring for aggregator’s Column import

Importing ai.chronon.aggregator.Column is correct, but since that case class lives in the aggregator module’s test sources, ensure your spark tests depend on it (aggregator % "test->test"). Otherwise the import will fail at test time.

  • Verify in your spark project definition (e.g. build.sbt or project/*.scala) there’s a .dependsOn(aggregator % "test->test") or equivalent Test -> Test mapping.
  • No accidental org.apache.spark.sql.Column import was found in DifferentPartitionColumnsTest.scala.
spark/src/test/scala/ai/chronon/spark/test/fetcher/FetcherTestUtil.scala (1)

3-3: No action needed: ai.chronon.aggregator.Column signature aligns with DataFrameGen usage
DataFrameGen imports Column from ai.chronon.aggregator and invokes it with three arguments—defaults for chunkSize and nullRate cover the remaining parameters. Tests will pass as-is.

spark/src/test/scala/ai/chronon/spark/test/join/JoinUtilsTest.scala (1)

19-19: Import path update verified; no stale imports.

  • No ai.chronon.aggregator.test.Column imports found.
  • case class Column lives under ai.chronon.aggregator in aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala.

LGTM.

spark/src/test/scala/ai/chronon/spark/test/join/SawtoothUdfPerformanceTest.scala (1)

5-5: LGTM: import path consolidation

The move from ai.chronon.aggregator.test to ai.chronon.aggregator for Column/NaiveAggregator/Timer/TRow looks correct and aligns with the PR objective.

spark/src/test/scala/ai/chronon/spark/test/join/BaseJoinTest.scala (1)

19-19: No lingering ai.chronon.aggregator.test.Column imports detected.
All tests now import the public ai.chronon.aggregator.Column.

api/src/test/scala/ai/chronon/api/DateMacroSpec.scala (1)

1-1: Package move is correct and necessary for private[api] access.

Placing the test in ai.chronon.api allows access to private[api] removeQuotesIfPresent. LGTM.

spark/src/test/scala/ai/chronon/spark/test/join/NoAggTest.scala (1)

19-19: Public Column import LGTM.

Consistent with the test migration to public API.

api/src/test/scala/ai/chronon/api/planner/LocalRunnerTest.scala (1)

1-1: Package relocation is appropriate.

Being in ai.chronon.api.planner removes the need for explicit LocalRunner import and aligns with the reorg.

api/src/test/scala/ai/chronon/api/DataPointerTest.scala (1)

1-1: Package move is consistent and correct.

Unqualified DataPointer/URIDataPointer references now resolve via package scope. LGTM.

spark/src/test/scala/ai/chronon/spark/test/join/HeterogeneousPartitionColumnsTest.scala (1)

19-19: Manual verification required: missing Column source

  • No Column.scala or Column.java found under ai/chronon/aggregator
  • Tests import ai.chronon.aggregator.Column but no definition was located
  • No stale ai.chronon.aggregator.test.Column imports found

Please confirm that Column is defined in the new package (and add or adjust the source file if needed) and update/remove imports accordingly.

spark/src/test/scala/ai/chronon/spark/test/batch/MergeJobVersioningTest.scala (1)

3-3: Switched to public Column import—LGTM

Matches the aggregator API surface used elsewhere in the PR.

api/src/test/scala/ai/chronon/api/CollectionExtensionsTest.scala (1)

1-3: Package move + local import resolve correctly

Using package ai.chronon.api and unqualified CollectionExtensions is appropriate here.

api/src/test/scala/ai/chronon/api/TilingUtilSpec.scala (1)

1-1: Package move aligns with reorg

No import needed for TilingUtils when in the same package.

spark/src/test/scala/ai/chronon/spark/test/analyzer/AnalyzerTest.scala (1)

19-19: Use of ai.chronon.aggregator.Column is consistent

No behavior change; compiles against the public aggregator API.

spark/src/test/scala/ai/chronon/spark/test/LocalTableExporterTest.scala (1)

19-19: Verify test-artifact dependency

LGTM: Switched to public Column import—avoids Spark’s Column conflict.

• Ensure the test artifact exposing ai.chronon.aggregator.Column is on spark-tests classpath
– Gradle: check testImplementation or sourceSets.test dependency on the aggregator project
– SBT/Maven: verify test->test dependsOn or a test-jar/classifier=tests artifact

spark/src/test/scala/ai/chronon/spark/test/StatsComputeTest.scala (1)

19-19: LGTM: Column import aligned with aggregator package.

Consistent with the test-utilities package relocation.

Confirm build wiring includes the aggregator test artifact if Column lives in test scope (see script in the previous comment).

spark/src/test/scala/ai/chronon/spark/test/join/EventsEventsTemporalTest.scala (1)

19-19: LGTM: Import migration is correct.

No behavioral change; resolves to the intended test Column.

Same classpath note: verify cross-module test dependency to aggregator test utilities (see script above).

api/src/test/scala/ai/chronon/api/planner/TableDependenciesTest.scala (1)

1-1: LGTM: Package moved into api.planner.

Allows unqualified access to TableDependencies; the explicit import is no longer needed.

spark/src/test/scala/ai/chronon/spark/test/streaming/AvroTest.scala (1)

19-19: LGTM: Column import updated to public API.

Matches the broader test import consolidation.

Ensure aggregator test utilities are available on the test classpath if Column is defined under test sources (see script above).

spark/src/test/scala/ai/chronon/spark/test/TestUtils.scala (1)

19-19: Import & Constructor Verified
No lingering ai.chronon.aggregator.test.Column imports found.
case class Column(name: String, type: DataType, cardinality: Int, chunkSize: Int = 10, nullRate: Double = 0.1) supports all three-arg usages in tests.

spark/src/test/scala/ai/chronon/spark/test/batch/ModularJoinTest.scala (1)

3-3: LGTM on switching to ai.chronon.aggregator.Column.
Consistent with the test-package consolidation; no behavioral changes expected.

api/src/test/scala/ai/chronon/api/DataTypeConversionTest.scala (1)

17-17: Package move to ai.chronon.api looks good.
Matches the suite-wide namespace reorg; test logic unchanged.

spark/src/test/scala/ai/chronon/spark/test/groupby/GroupByUploadTest.scala (1)

19-19: Import update to ai.chronon.aggregator.Column is appropriate.
Keeps tests on the public data-gen API; no other changes needed here.

spark/src/test/scala/ai/chronon/spark/test/batch/ShortNamesTest.scala (1)

3-3: Switch to ai.chronon.aggregator.Column is consistent and safe.
Matches the repo-wide test import consolidation.

spark/src/test/scala/ai/chronon/spark/test/join/SelectedJoinPartsTest.scala (1)

19-19: No lingering old Column imports
Verified across all Scala tests: no ai.chronon.aggregator.test.Column or ai.chronon.api.test imports remain.

api/src/test/scala/ai/chronon/api/TimeExpressionSpec.scala (1)

1-3: LGTM: package move + direct member import

Package ai.chronon.api and importing ColumnExpression.getTimeExpression are correct; unqualified Query/ColumnExpression resolve in-package.

api/src/test/scala/ai/chronon/api/planner/ConfPlannerTest.scala (1)

1-1: LGTM: test colocated under planner package

Localizing tests in ai.chronon.api.planner removes redundant imports and matches API surface.

spark/src/test/scala/ai/chronon/spark/test/batch/MergeJobAnalyzeReuseTest.scala (1)

3-3: LGTM: use public Column

Consistent with the PR-wide migration off ai.chronon.aggregator.test.Column.

api/src/test/scala/ai/chronon/api/RelevantLeftForJoinPartSpec.scala (1)

1-1: Package move LGTM.

Namespace aligns with the broader test reorg; no logic changes.

spark/src/test/scala/ai/chronon/spark/test/DataFrameGen.scala (1)

19-19: No lingering deprecated test packages found

Ran the search for ai.chronon.(aggregator|api).test – no matches. Good to merge.

spark/src/test/scala/ai/chronon/spark/test/MetadataExporterTest.scala (1)

19-19: Import switch to ai.chronon.aggregator.Column is valid.

Consistent with the rest of the refactor.

api/src/test/scala/ai/chronon/api/ExtensionsTest.scala (1)

17-23: Package/import cleanup LGTM.

Local wildcard imports (Extensions., ScalaJavaConversions.) and trimmed JUnit asserts are appropriate; tests still compile with used asserts only.

api/src/test/scala/ai/chronon/api/planner/StagingQueryPlannerTest.scala (1)

1-1: Package + import reorg LGTM.

Using ai.chronon.api._ and Builders for StagingQuery/MetaData simplifies references; LocalRunner/StagingQueryPlanner resolve via package.

Also applies to: 4-4

api/src/test/scala/ai/chronon/api/planner/MonolithJoinPlannerTest.scala (2)

1-1: Package move to api.planner looks good

Local references to MonolithJoinPlanner/LocalRunner resolve via same package. No issues spotted.


5-5: Extensions wildcard import OK

Needed for implicit ops; placement at top is fine.

api/src/test/scala/ai/chronon/api/planner/MetaDataUtilsTest.scala (1)

1-3: Package/import consolidation LGTM

Moving tests to api.planner and using ai.chronon.api._ simplifies usage. Looks consistent with the PR.

api/src/test/scala/ai/chronon/api/TileSeriesSerializationTest.scala (1)

1-5: Package move and import simplification LGTM

Using local package access for Constants/ThriftJsonCodec and consolidating imports is clean.

Also applies to: 9-9

api/src/test/scala/ai/chronon/api/planner/GroupByPlannerTest.scala (1)

1-1: Package move + wildcard import LGTM

Matches the test reorg and reduces explicit imports.

Also applies to: 5-5

aggregator/src/test/scala/ai/chronon/aggregator/ApproxDistinctTest.scala (1)

17-17: Package move to ai.chronon.aggregator LGTM

No behavior changes; tests remain intact.

aggregator/src/test/scala/ai/chronon/aggregator/UniqueTopKTest.scala (1)

1-1: Package move to ai.chronon.aggregator: OK.

Namespace update only; tests should behave identically.

aggregator/src/test/scala/ai/chronon/aggregator/FrequentItemsTest.scala (1)

1-1: Package move to ai.chronon.aggregator: OK.

Pure namespace change; no functional impact.

spark/src/test/scala/ai/chronon/spark/test/join/SkipBloomFilterJoinBackfillTest.scala (2)

21-21: api wildcard import change is fine; keep both alias and wildcard.

Both import ai.chronon.api (alias for api.StringType usage) and import ai.chronon.api._ are needed here.


19-19: Verify test dependency between Spark and Aggregator
No build definitions were found to confirm a test→test link. Please ensure that in your build configuration the spark test classpath includes aggregator:test to avoid NoClassDefFoundError.

• Double-check your build tool’s project file (e.g. build.sbt, pom.xml, etc.) for a test->test or equivalent dependency from the Spark module to the Aggregator test sources.

aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala (1)

17-17: Package move to ai.chronon.aggregator: OK.

Aligns with test reorg; no behavior change.

spark/src/test/scala/ai/chronon/spark/test/join/EventsEntitiesSnapshotTest.scala (1)

19-19: Import path change to public Column looks good

Switching to ai.chronon.aggregator.Column aligns with the package move.

aggregator/src/test/scala/ai/chronon/aggregator/MinHeapTest.scala (1)

17-17: Package shift LGTM

Moving tests into ai.chronon.aggregator is consistent with the broader reorg.

spark/src/test/scala/ai/chronon/spark/test/join/KeyMappingOverlappingFieldsTest.scala (1)

19-19: Import path change to public Column looks good

Consistent with the test package relocation.

aggregator/src/test/scala/ai/chronon/aggregator/ApproxPercentilesTest.scala (1)

17-17: Package shift LGTM

No logic changes; namespace update only.

aggregator/src/test/scala/ai/chronon/aggregator/RowAggregatorTest.scala (1)

17-17: Package shift LGTM

Namespace moved to ai.chronon.aggregator; tests remain unchanged.

aggregator/src/test/scala/ai/chronon/aggregator/VarianceTest.scala (1)

17-17: Package move LGTM.
Namespace shift to ai.chronon.aggregator is consistent with the PR’s goal; no logic changes.

aggregator/src/test/scala/ai/chronon/aggregator/EditDistanceTest.scala (1)

17-17: Package rename looks good.
Tests remain unchanged; this aligns with the suite-wide move.

aggregator/src/test/scala/ai/chronon/aggregator/SawtoothOnlineAggregatorTest.scala (2)

17-17: Package move LGTM.
Consistent with relocating tests into the main package.


17-20: No lingering ai.chronon.aggregator.test references found
Regex search confirms no occurrences in package declarations or imports.

aggregator/src/test/scala/ai/chronon/aggregator/NaiveAggregator.scala (1)

17-17: No FQN collision for NaiveAggregator
The only definition is in src/test—no matching class/object in production.

aggregator/src/test/scala/ai/chronon/aggregator/MomentTest.scala (1)

1-1: Package rename approved.
No functional changes; consistent with the suite-wide reorg.

aggregator/src/test/scala/ai/chronon/aggregator/TwoStackLiteAggregatorTest.scala (1)

17-17: Package move to ai.chronon.aggregator looks correct.

Aligns with the broader test reorg. No functional impact.

aggregator/src/test/scala/ai/chronon/aggregator/SawtoothAggregatorTest.scala (1)

17-17: Stale-imports check passed

No occurrences of ai.chronon.aggregator.test remain.

spark/src/test/scala/ai/chronon/spark/test/bootstrap/BootstrapUtils.scala (1)

19-19: Public Column import LGTM—no stale test‐only refs or spark shadowing

  • No occurrences of ai.chronon.aggregator.test.Column.
  • No org.apache.spark.sql.Column import in BootstrapUtils.scala.
    Please verify that the public ai.chronon.aggregator.Column constructor signature matches its use here.
spark/src/test/scala/ai/chronon/spark/test/StagingQueryTest.scala (1)

19-19: LGTM: Column import and signature verified

  • No lingering ai.chronon.aggregator.test.Column imports
  • aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala’s case class Column includes nullRate

package ai.chronon.spark.test.batch

import ai.chronon.aggregator.test.Column
import ai.chronon.aggregator.Column
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify cross-module test visibility for Column

Column appears to be defined under aggregator test sources (see aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala:173-203). Ensure spark tests still depend on aggregator’s test-jar; otherwise this import will fail.

Run this to confirm repository consistency:

If needed, add to your SBT settings:

lazy val spark = project
  .dependsOn(aggregator % "test->test")

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "1) Check for leftover old imports:"
rg -nP -C1 '\bai\.chronon\.aggregator\.test\.Column\b' || echo "OK: no old ai.chronon.aggregator.test.Column imports"

echo
echo "2) Locate Column definitions under ai.chronon.aggregator:"
rg -nPU --type=scala -n -C2 '(?s)^\s*package\s+ai\.chronon\.aggregator\b.*?\bcase\s+class\s+Column\s*\(' || true

echo
echo "3) Check SBT for test->test dependency (aggregator test-jar visibility):"
rg -nP -C3 'test->test' -g 'build.sbt' -g '**/build.sbt' -g 'project/**' || echo "Note: no explicit test->test dependency found. If Column is only in aggregator test sources, add: dependsOn(aggregator % \"test->test\")."

Length of output: 17711


Ensure spark tests can import Column from aggregator’s test sources

  • Detected case class Column only in aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala:174.
  • No test->test dependency found between spark and aggregator.
  • In your SBT settings (e.g. build.sbt), add:
lazy val spark = project
  .dependsOn(aggregator % "test->test")

This guarantees spark’s tests can resolve ai.chronon.aggregator.Column.

🤖 Prompt for AI Agents
In spark/src/test/scala/ai/chronon/spark/test/batch/EvalTest.scala around line
3, the test imports ai.chronon.aggregator.Column which is defined only in
aggregator's test sources but there is no test->test dependency from spark to
aggregator; update the SBT project settings (e.g. build.sbt or project build
file) to add a test-scoped dependency so that spark tests depend on aggregator's
test sources (for example, make the spark subproject dependOn aggregator %
"test->test"), then re-run the test compilation to verify the import resolves.

Comment on lines +19 to 20
import ai.chronon.aggregator.Column
import ai.chronon.api
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify cross-module test dependency for Column

Column is defined in aggregator test scope. Ensure spark tests can see aggregator’s test-jar (Test->Test dep or equivalent).


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "1) Locate Column definition (should be under aggregator test sources)"
rg -n -C2 -g '!**/target/**' -P '(?m)^\s*case\s+class\s+Column\b'

echo
echo "2) Check SBT-style cross-module Test->Test dependency on aggregator"
rg -n -C2 -g '!**/target/**' -P 'dependsOn\([^\)]*aggregator[^\)]*%[^\)]*Test\s*->\s*Test'

echo
echo "3) Check if aggregator publishes test artifacts (SBT)"
rg -n -C2 -g '!**/target/**' -P '(publishArtifact\s+in\s+Test\s*:=\s*true|Test\s*/\s*publishArtifact\s*:=\s*true)'

echo
echo "4) If Maven is used, check for test-jar publication"
fd -a -t f pom.xml | while read -r pom; do
  echo "Scanning $pom"
  rg -n -C2 -P '<goal>\s*test-jar\s*</goal>|maven-jar-plugin|test-jar' "$pom" || true
done

echo
echo "If step 2 and 3/4 return no matches, add aggregator as a Test->Test dependency to the spark module or publish/consume aggregator's test-jar."

Length of output: 1096


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "3) Check if aggregator publishes test artifacts (SBT)"
rg -n -C2 -g '!**/target/**' -P '(publishArtifact\s+in\s+Test\s*:=\s*true|Test\s*/\s*publishArtifact\s*:=\s*true)'

echo
echo "4) Check for test-jar publication in any pom.xml"
fd -a -t f pom.xml | while read -r pom; do
  echo "Scanning $pom"
  rg -n -C2 -P '<goal>\s*test-jar\s*</goal>|maven-jar-plugin|<testJar>' "$pom" || true
done

Length of output: 253


Add missing Test→Test dependency for Column
The Column case class lives in aggregator test sources, but we found no SBT dependsOn(... % "Test->Test") or test-jar publication. Without this, spark’s tests won’t compile.

• In the Spark module’s build definition (e.g. spark/build.sbt), add:

.lazy val spark = (project in file("spark"))
  .dependsOn(aggregator % "test->test")

—or, if you prefer Maven, configure the aggregator module to publish a test-jar and declare it as a <scope>test</scope> dependency in Spark’s pom.xml.

🤖 Prompt for AI Agents
In
spark/src/test/scala/ai/chronon/spark/test/join/EventsEntitiesSnapshotTest.scala
around lines 19-20, the test references Column from the aggregator test sources
but the Spark module does not declare a Test→Test dependency on aggregator,
causing compilation failures; update the Spark build to depend on aggregator's
test sources by adding a test-scope dependency—if using SBT, add
.dependsOn(aggregator % "test->test") to the spark project definition (e.g. in
spark/build.sbt); if using Maven, configure the aggregator module to publish a
test-jar and add that test-jar as a <scope>test</scope> dependency in Spark’s
pom.xml.

package ai.chronon.spark.test.join

import ai.chronon.aggregator.test.Column
import ai.chronon.aggregator.Column
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify Column switch doesn’t break DataFrameGen usage

Switched to ai.chronon.aggregator.Column. Ensure this is the test helper with the expected constructor and generators (or an equivalent main API) and that spark tests can see it (test->test dep if it lives under aggregator/src/test). Otherwise, this will fail at compile/run-time.

Run to confirm Column’s location and signature, and whether it’s only in test sources:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Where is Column defined?"
rg -nP -C3 'case\s+class\s+Column\s*\('

echo
echo "Definitions under src/main:"
rg -nP 'case\s+class\s+Column\s*\(' -g '*/src/main/**' || true

echo
echo "Definitions under src/test:"
rg -nP 'case\s+class\s+Column\s*\(' -g '*/src/test/**' || true

echo
echo "DataFrameGen.entities signature (to see expected Column usage):"
fd -a DataFrameGen.scala | xargs -I{} rg -n -C3 'def\s+entities\s*\(' {}

Length of output: 1746


Ensure Column is available to Spark tests

The Column case class is only declared under
aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala:174
and isn’t part of the aggregator’s main artifact. Spark’s tests import ai.chronon.aggregator.Column, but without a test-jar dependency on aggregator, this will fail to compile/run.

• Move Column (and any related helpers) to aggregator/src/main/scala
OR
• Publish aggregator’s test-jar and add it as a test dependency in the Spark module

🤖 Prompt for AI Agents
In
spark/src/test/scala/ai/chronon/spark/test/join/NoHistoricalBackfillTest.scala
around line 19, the test imports ai.chronon.aggregator.Column which currently
lives only in aggregator/src/test/scala (DataGen.scala) and is not on the Spark
test classpath; either move the Column case class (and any closely related
helpers it needs) from aggregator/src/test/scala into aggregator/src/main/scala
under the same package so it’s published in the aggregator main artifact, update
any imports/usages accordingly, and remove the test-only definition; OR keep it
in aggregator tests but publish aggregator’s test-jar and add that test-jar as a
test dependency of the Spark module (update aggregator build to enable
publishing test JAR and add the test dependency in Spark’s build.sbt/maven
config) so the Spark tests can compile.

package ai.chronon.spark.test

import ai.chronon.aggregator.test.Column
import ai.chronon.aggregator.Column
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify no lingering legacy imports and test-fixture availability

Ensure no ai.chronon.aggregator.test.* imports remain and that spark tests can access aggregator test fixtures (Column lives under aggregator test sources).


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "[1/2] Searching for legacy '.aggregator.test' imports (expect: none)"
rg -nP -C2 '\bai\.chronon\.aggregator\.test(?:\.\w+)?\b' || echo "OK: none found"

echo "[1/2] Confirming Column case class exists under ai.chronon.aggregator (expect: ≥1 hit)"
rg -nPU --type=scala '(?s)^\s*package\s+ai\.chronon\.aggregator\b.*?\bcase\s+class\s+Column\b' || echo "WARN: Column not found under ai.chronon.aggregator"

echo "[2/2] Checking sbt for Test->Test dependency to expose aggregator test fixtures to spark tests (expect: at least one hit)"
fd -t f -e sbt | xargs -r rg -nP -C2 'test->test|Test->Test|%[[:space:]]*"test->test"' || echo "NOTE: No explicit Test->Test dep found; ensure test fixtures are on test classpath."

Length of output: 16958


Expose aggregator test fixtures to spark tests

  • No legacy ai.chronon.aggregator.test imports remain.
  • Column is defined under aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala.
  • Update your SBT build so that the Spark test project has a test->test dependency on the aggregator test project (e.g. add in build.sbt) to expose Column.
🤖 Prompt for AI Agents
In spark/src/test/scala/ai/chronon/spark/test/MigrationCompareTest.scala around
line 19, the test currently imports Column from ai.chronon.aggregator which no
longer exposes the test fixture; Column lives in the aggregator test sources
(aggregator/src/test/scala/ai/chronon/aggregator/DataGen.scala). Update the
import to the correct package that DataGen/Column uses (e.g.
ai.chronon.aggregator.test or the exact package declared in DataGen.scala) and
remove any legacy ai.chronon.aggregator.test references left elsewhere; then
modify build.sbt to add a test->test dependency from the Spark test project to
the aggregator test project (project("spark") dependsOn(project("aggregator") %
"test->test")) so the aggregator test fixtures are visible to Spark tests.

## Summary

## Checklist
- [ ] Added Unit Tests
- [ ] Covered by existing CI
- [ ] Integration tested
- [ ] Documentation update


<!-- av pr metadata
This information is embedded by the av CLI when creating PRs to track
the status of stacks when using Aviator. Please do not delete or edit
this section of the PR.
```
{"parent":"tchow/repackage","parentHead":"b3423ab78d52f6f8f3eea1681a387eaeebb9f717","parentPull":1083,"trunk":"main"}
```
-->

Co-authored-by: thomaschow <[email protected]>
@tchow-zlai tchow-zlai closed this Sep 7, 2025
@tchow-zlai tchow-zlai deleted the tchow/repackage branch September 7, 2025 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants