Skip to content

[SPARK-33079][TESTS] Replace the existing Maven job for Scala 2.13 in Github Actions with SBT job #29958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sarutak wants to merge 6 commits into from
Closed

[SPARK-33079][TESTS] Replace the existing Maven job for Scala 2.13 in Github Actions with SBT job #29958

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8e45bc8
Added GitHub Actions job for building Spark with SBT for Scala 2.13.
sarutak Oct 6, 2020
5a22c38
Fix build_and_test.yml to consider .m2, test:compile and local cache …
sarutak Oct 6, 2020
5dec1ff
Merge branch 'master' of https://github.com/apache/spark into add-sbt…
sarutak Oct 6, 2020
47a4806
Replaced Maven test with SBT test for Scala 2.13.
sarutak Oct 9, 2020
91d1066
Merge branch 'master' of https://github.com/apache/spark into add-sbt…
sarutak Oct 9, 2020
30b2ddd
Changed the build goal to compile and test:compile.
sarutak Oct 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/workflows/build_and_test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,3 +319,29 @@ jobs:
./dev/change-scala-version.sh 2.13
./build/mvn $MAVEN_CLI_OPTS -DskipTests -Pyarn -Pmesos -Pkubernetes -Phive -Phive-thriftserver -Phadoop-cloud -Djava.version=11 -Pscala-2.13 install
rm -rf ~/.m2/repository/org/apache/spark

scala-213-sbt:
Comment thread
HyukjinKwon marked this conversation as resolved.
Outdated
name: Scala 2.13 build with SBT

@HyukjinKwon HyukjinKwon Oct 7, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hm, should we set this job in the PR builder? The (almost) same thing is tested at "Scala 2.13 build". I think we'll set a Jenkins regular job to run the tests at SPARK-33044 as well. I would avoid adding too many jobs here since we're not able to have more resources (see also SPARK-32264).

@sarutak sarutak Oct 7, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The (almost) same thing is tested at "Scala 2.13 build"

The main purpose of adding the build with SBT is not just testing with SBT. As I mentioned, some compilation error happens only with SBT so I'd like detect such error before any changes being merged.
I understand we have not enough resource for GitHub Action but if an additional build is added to the Jenkins regular job, it will take much more longer time doesn't it?
Are there any problem to replace the existing "Scala 2.13 build" with the new one?

@HyukjinKwon HyukjinKwon Oct 7, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am okay with replacing the existing "Scala 2.13 build". Seems that'll catch the compilation errors more correctly.

it will take much more longer time doesn't it?

It does takes longer to detect the errors. However, there are many other combinations to test and currently we can't test all combinations. So I think the default common-ish combination was picked to use it as a GitHub Actions and Jenkins PR builder. For example, JDK 11 is not being tested in GitHub Actions and Jenkins PR builder.

@sarutak sarutak Oct 7, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For example, JDK 11 is not being tested in GitHub Actions and Jenkins PR builder.

Yeah, I know but whether build is successfully finish is tested in GitHub Actions so I thought build test (not unit test) should be checked in GitHub Actions too for now.
Anyway, If we can replace the existing "Scala 2.13 build" with the new one, I'll do it.
@dongjoon-hyun Do you have any concern about replacing it?

Comment thread
HyukjinKwon marked this conversation as resolved.
Outdated
runs-on: ubuntu-latest
steps:
- name: Checkout Spark repository
uses: actions/checkout@v2
- name: Cache Ivy local repository
uses: actions/cache@v2
with:
path: ~/.ivy2/cache
key: scala-213-ivy-${{ hashFiles('**/pom.xml', '**/plugins.sbt') }}
restore-keys: |
scala-213-ivy-
- name: Install Java 11
uses: actions/setup-java@v1
with:
java-version: 11
- name: Build with SBT
run: |
mkdir -p ~/.ivy2/local
mkdir -p ~/.m2
Comment thread
HyukjinKwon marked this conversation as resolved.
Outdated
./dev/change-scala-version.sh 2.13
./build/sbt -Pyarn -Pmesos -Pkubernetes -Phive -Phive-thriftserver -Phadoop-cloud -Pkinesis-asl -Djava.version=11 -Pscala-2.13 test:compile publishLocal
rm -rf ~/.ivy2/local/org.apache.spark
rm -rf ~/.m2/repository/org/apache/spark
2 changes: 1 addition & 1 deletion ...kinesis-asl/src/main/scala/org/apache/spark/streaming/kinesis/KinesisBackedBlockRDD.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class KinesisBackedBlockRDD[T: ClassTag](
require(_blockIds.length == arrayOfseqNumberRanges.length,
"Number of blockIds is not equal to the number of sequence number ranges")

override def isValid(): Boolean = true
override def isValid: Boolean = true

@LuciferYang LuciferYang Oct 9, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this change related to the current pr?

@sarutak sarutak Oct 9, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes. empty parameter list causes compilation error if we build for Scala 2.13 with SBT.
This is what I'd like to detect by CI.


override def getPartitions: Array[Partition] = {
Array.tabulate(_blockIds.length) { i =>
Expand Down