-
Notifications
You must be signed in to change notification settings - Fork 84
Bazel migration phase 2 #911
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.
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
Conversation
b8ca5f9 to
9defeee
Compare
9defeee to
ae9e3a7
Compare
|
Will Bazel support all kinds of unit test frameworks like Junit, TestNG? |
lakshmanayenduri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to add Verify statements in your test, not to just rely on asserts
| def testDecimal(): Unit = { | ||
|
|
||
| println("inside test") | ||
| val namespace = "test_decimal" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need println statement here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will remove it, good catch
lakshmanayenduri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please review my comments
jvm/spark_3_1_repo.bzl
Outdated
| "org.apache.datasketches:datasketches-java:2.0.0", | ||
| "org.apache.datasketches:datasketches-memory:1.3.0", | ||
| "org.apache.hive:hive-exec:3.1.2", | ||
| "org.apache.hive:hive-exec:2.3.7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we downgrading the hive version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for spark 3.1, it requires to downgrade to compatible hive version
jvm/spark_3_5_repo.bzl
Outdated
| # Other dependencies | ||
| "org.apache.curator:apache-curator:2.12.0", | ||
| "com.esotericsoftware:kryo:5.1.1", | ||
| # "com.esotericsoftware:kryo:5.6.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the space between # and "
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you, will do
|
This PR has been open for sometime, can we please have an ETA for this? I am actually waiting to test this, seems Uber guys are waiting too. |
Thanks Laxman, I was on vacation. let me finish this by next week |
I will let @tswitzer-netflix to take over fixing unit tests with his PR: #991. thank you Tom. |
Summary
Bazel migration phase 2
Why / Goal
Test Plan
Checklist
Reviewers