You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "cloud-fan (via GitHub)" <gi...@apache.org> on 2026/01/23 12:47:00 UTC

[PR] [SPARK-55021][CORE][TESTS] Move timezone/locale initialization to constructor in SparkTestSuite [spark]

cloud-fan opened a new pull request, #53934:
URL: https://github.com/apache/spark/pull/53934

   ### What changes were proposed in this pull request?
   
   Move the timezone and locale initialization from `beforeAll()` to the trait's constructor body in `SparkTestSuite`.
   
   ### Why are the changes needed?
   
   This is a follow-up fix to #53784 which introduced `SparkTestSuite`.
   
   The original PR moved timezone/locale initialization from the class body to `beforeAll()`. This causes test failures when running tests like `RowJsonSuite` with Maven, because tests that create timezone-sensitive objects (like `java.sql.Timestamp`) during test registration fail - the timezone isn't set until `beforeAll()` runs, which happens after class construction.
   
   For example, in `RowJsonSuite`:
   ```scala
   testJson(
     Timestamp.valueOf("2017-01-06 10:22:03.00"),  // Created during class construction
     TimestampType,
     JString("2017-01-06 10:22:03"))
   ```
   
   `Timestamp.valueOf()` interprets the string in the current default timezone. If the system timezone is not `America/Los_Angeles`, the Timestamp represents a different instant, causing test failures.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No, test-only
   
   ### How was this patch tested?
   
   ```
   build/mvn test -pl sql/catalyst -DwildcardSuites=org.apache.spark.sql.RowJsonSuite
   ```
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Yes


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-55021][CORE][TESTS][FOLLOWUP] Move timezone/locale initialization to constructor in SparkTestSuite [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang closed pull request #53934: [SPARK-55021][CORE][TESTS][FOLLOWUP] Move timezone/locale initialization to constructor in SparkTestSuite
URL: https://github.com/apache/spark/pull/53934


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-55021][CORE][TESTS] Move timezone/locale initialization to constructor in SparkTestSuite [spark]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #53934:
URL: https://github.com/apache/spark/pull/53934#issuecomment-3790085494

   ## JIRA Issue Information
   
   === Sub-task SPARK-55021 ===
   Summary: Introduce SparkTestSuite as the base class for all scala tests
   Assignee: Ruifeng Zheng
   Status: Resolved
   Affected: ["4.2.0"]
   
   ---
   *This comment was automatically generated by GitHub Actions*


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-55021][CORE][TESTS][FOLLOWUP] Move timezone/locale initialization to constructor in SparkTestSuite [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #53934:
URL: https://github.com/apache/spark/pull/53934#issuecomment-3797673567

   Merged into master. Thanks @cloud-fan and @zhengruifeng 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-55021][CORE][TESTS][FOLLOWUP] Move timezone/locale initialization to constructor in SparkTestSuite [spark]

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #53934:
URL: https://github.com/apache/spark/pull/53934#issuecomment-3790398946

   thanks for the quick fix
   
   @LuciferYang 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-55021][CORE][TESTS][FOLLOWUP] Move timezone/locale initialization to constructor in SparkTestSuite [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #53934:
URL: https://github.com/apache/spark/pull/53934#issuecomment-3797704801

   start a new Maven test for verification: https://github.com/apache/spark/actions/runs/21344475845


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org