Conversation
I mean that sounds like a big deal. |
|
Running pytest on the original test takes 55s. |
f1af3ed to
db5885e
Compare
|
After rebase, it seems like it doesn't affect CI runtime. |
Add local = True
db5885e to
a97bd92
Compare
Szelethus
left a comment
There was a problem hiding this comment.
It is kind of terrible how much boilerplate is needed to migrate this to bazel. And its especially a shame how much code we are seemingly duplicating from other tests. Is this absolutely necessary?
| parser.add_argument( | ||
| "--srcs", | ||
| nargs="*", | ||
| default=[], | ||
| help="Source files to copy into the workspace root", | ||
| ) |
| parser.add_argument( | ||
| "--expected_exit_code", | ||
| type=int, | ||
| default=0, | ||
| help="Expected exit code from the bazel command", | ||
| ) |
|
|
||
| external_test( | ||
| name = "compile_commands_isystem_test", | ||
| action = "build", |
There was a problem hiding this comment.
How integral is this to this test?
| parser.add_argument( | ||
| "--action", | ||
| required=True, | ||
| choices=["build", "test"], | ||
| help="Bazel action to run", | ||
| ) |
There was a problem hiding this comment.
This is technically used, but I'm sure there is a point.
| # Write empty WORKSPACE | ||
| write_file( | ||
| os.path.join(tmpdir, "WORKSPACE"), | ||
| "# This file is mandatory for old Bazel versions\n", |
There was a problem hiding this comment.
I mean, thats not the real explanation. We only support bazel 7 and up where its not a requirement, nor is it the default.
| action, | ||
| target, | ||
| "--experimental_cc_implementation_deps", | ||
| "--enable_bzlmod", |
There was a problem hiding this comment.
So we are testing here with bzlmod and workspace coexisting?
| def cleanup(tmpdir): | ||
| """Shut down bazel server to release file locks.""" | ||
| subprocess.run( | ||
| ["bazel", "shutdown"], | ||
| cwd=tmpdir, | ||
| capture_output=True, | ||
| check=False, | ||
| ) |
There was a problem hiding this comment.
Is this a common issue you are experiencing?
So you mentioned in person this is likely due to bazelisk re-downloading bazel for each target? |
Why:
We want all tests to be run with
bazel test //...What:
Addresses:
#210