Bazel testing fixes #179
Open
Bazel testing fixes #179
+217
−29
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
arkjedrz
suggested changes
Nov 28, 2025
| load("@score_tooling//python_basics:defs.bzl", "score_py_pytest", "score_virtualenv") | ||
|
|
||
| # Additional requirements for the tests | ||
| # In order to update the requirements, change the `requirements.in` file and run: |
Contributor
There was a problem hiding this comment.
That's a nitpick, but:
# In order to update the `requirements.txt` file, modify the `requirements.in` file and run:
# `bazel run //tests/python_test_cases:requirements.update`.
#
# To upgrade all dependencies to their latest versions, run:
# `bazel run //tests/python_test_cases:requirements.update -- --upgrade`.
I also think such info might be helpful in README.md.
| @@ -1,14 +1,5 @@ | |||
| # Fully resolved list of dependencies is required by Bazel. | |||
Contributor
There was a problem hiding this comment.
This is the fully resolved list? Why versions are not longer provided?
Contributor
Author
There was a problem hiding this comment.
Our previous usage of requirements file was incorrect. Now when we generate the lockfile it is provided to bazel. Lockfile has all dependencies resolved so we could get rid of frozen versions and cluttered list.
Removed comment.
BUILD
Outdated
| test_suite( | ||
| name = "unit_tests", | ||
| tests = [ | ||
| # C++ |
Contributor
There was a problem hiding this comment.
Redundant comments? You can infer from the target name, same as You'd do with cit_tests targets.
| name = "unit_tests", | ||
| tests = [ | ||
| # C++ | ||
| "test_kvs_cpp", |
Contributor
There was a problem hiding this comment.
This might not require changes, but I'm curious - why C++ UTs do not require full target name?
Contributor
Author
There was a problem hiding this comment.
test_kvs_cpp is a test suite. Test suites in bazel can be nested.
PiotrKorkus
force-pushed
the
piotrkorkus_bazel_testing
branch
from
d59c9a0 to
631876c
Compare
arkjedrz
approved these changes
Nov 28, 2025
venv setup was incorrect args were not always recognized
add common test suites across repositories
PiotrKorkus
force-pushed
the
piotrkorkus_bazel_testing
branch
from
631876c to
9bb83ed
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
reference_integrationUTs from
score_persistencyscore#2244