-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[java] add missing bazel artifacts #16773
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
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||||||
PR Code Suggestions ✨No code suggestions found for the PR. |
asolntsev
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.
@titusfortner Thank you for the fix!
P.S. In a standard Gradle or Maven project, this problem would never happen ;)
|
Right, other problems would happen. I appreciate your help fixing up places in the code that need it, but it's not helpful to complain about choices that have made that aren't going to change. In this case the CI displayed the error message unable to build all targets, and listed above the 4 targets that didn't build; running the target locally said exactly what needed to be changed. |
My bad, next time I will ask for help if I cannot find the right error message.
No. No. No. Sorry, I don't buy this argument. While people don't like the chosen, not-so-standard, not-so-widely-used tool, you will hear complains. It seems totally logical to me ;) |
|
Right, but complaining isn't going to change anything, it only makes you look bad and everyone else frustrated/exhausted. I get that bazel isn't the preferred solution for most people seeing it for the first time, but it has some significant benefits beyond what the more common java build systems provide (feel free to research them), and we simply aren't going to redo everything we have based on someone complaining one more time about it. 😄 |
|
@titusfortner Yes, this sounds totally reasonable. I agree. |
|
Be part of the solution not part of the problem 😉 Also, Simon wrote about some of the advantages of Bazel here: https://www.selenium.dev/blog/2023/building-selenium/ |
User description
🔗 Related Issues
Fixes broken tests from #16765
PR Type
Bug fix
Description
Add missing AssertJ dependency to Bazel build configurations
Fixes broken tests from PR [refactor] replace JUnit assertions by AssertJ #16765
Updates two test target BUILD files with required artifact
Diagram Walkthrough
File Walkthrough
BUILD.bazel
Add AssertJ dependency to test-base targetjava/test/org/openqa/selenium/environment/BUILD.bazel
artifact("org.assertj:assertj-core")dependency totest-base target
BUILD.bazel
Add AssertJ dependency to SmallTests targetjava/test/org/openqa/selenium/testing/BUILD.bazel
artifact("org.assertj:assertj-core")dependency toSmallTests target