WIP: Consumer tests#129
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //src:license-checkStatus: ✅ Passed Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
There was a problem hiding this comment.
Pull Request Overview
This draft adds end-to-end consumer tests that clone sample repositories, override their Bazel dependencies (locally or via Git), and verify build success.
- Introduces a
ConsumerRepodataclass and a list of repos to test - Implements helper functions to patch
MODULE.bazelfor local and Git overrides - Adds a single parametrized test (
test_and_clone_repos) to clone repos, apply overrides, and run Bazel commands
Comments suppressed due to low confidence (1)
src/tests/test_consumer.py:24
- [nitpick] Attribute names should follow snake_case; rename to
local_override_resultfor consistency.
LocalOverrideResult: bool
1e2840e to
5a944e8
Compare
| # Running through all 'cmds' specified with the local override | ||
| print_running_cmd(repo.name, cmd, "LOCAL OVERRIDE") | ||
|
|
||
| out = subprocess.run(cmd.split(), capture_output=True, text=True) |
There was a problem hiding this comment.
why not check the subprocess for errors also here? with check=True. I feel like if some future maintainer “optimises” the processing of BR and forgets to look at returncode failures silently become successes.
There was a problem hiding this comment.
The issue is, that if I do 'check=True' here, if bazel exits with a non 0 exit code (which it does when sphinx errors for example) it just stops execution immediatly there.
At least to my testing. That's why I thought it better to not do this here.
There was a problem hiding this comment.
it throws an exception. you could catch it. It's safer - but more boilerplate than checking return code.
|
Also note. This is on purpose not a Bazel test target, as this is executed via direct call. Additionally, the placement of this is obviously not finaly, I just put it there for now, if you can think of where it would fit better, let me know. |
b92023a to
12328cc
Compare
|
@AlexanderLanin Where should this test file be located btw? Not really sure if this is a good place for it, where I put it currently. |
|
One thing I do struggle with this, and have not even attempted is. How can this be tested beyond just manually testing it and making sure it does what it should? |
12328cc to
89fba1c
Compare
|
Don't forget to correct the formatting check |
Thanks. Just pushed the formatting. |
7875a46 to
193945a
Compare
Adding some test commands
Figured out why it wasn't producing wanted errors Now need to introduce error parsing
Found a way to get results out of stderr&out. Still needs quiet a bit of work. But slowly getting there.
Removed Metamodel exception. Simplified code structure.
7635561 to
42011e1
Compare
Developing the consumer tests here for docs-as-code.
Just want to gather some feedback here on the general appraoch etc.