Repository navigation
[debug-tools/rocr-debug-agent] Enable rocr-debug-agent CI tests - #2859
Conversation
6d4aae5 to
a67f5a1
Compare
7e9ba03 to
3077ce8
Compare
a67f5a1 to
842bba7
Compare
|
This could be an opportunity to include the python script being called through a module import or similar, instead of a python launch within this python script via subprocess. |
Ok. But that requires some rework in debug agent. Let me get the tests running reliably in CI first, and then I can look into that. |
9b63e5e to
3b5b9a2
Compare
3b5b9a2 to
1960925
Compare
|
I haven't yet explored the python module idea as that requires changes to the rocr-debug-agent itself (the run_test.py script). I think we could do this post-migration to rocm-systems. Or even before. But I wanted to get tests going sooner rather than later. |
1960925 to
88c95f6
Compare
9d291b9 to
a676387
Compare
lumachad
changed the title
lumachad
changed the title
a676387 to
a640efa
Compare
a640efa to
5383305
Compare
|
I have reworked the testing script a little. Mostly refactoring, removing emojis, formatting output etc. This script only tests the rocr-debug-agent. |
| working_dir: Path, | ||
| test_bin_dir: Path, | ||
| env_vars: Optional[Dict[str, str]] = None, | ||
| max_retries: int = 3, |
There was a problem hiding this comment.
let's avoid retries at initial implementation.
if we see flaky issues that need retries, we can add it then but let's get tests working and actually see its behavior
There was a problem hiding this comment.
I'd rather keep it. The reasoning here is we definitely don't want to fail other components' PR's due to random timeouts on busy GPU's at the time of testing. I've checked this in practice and I think it will work to our favor.
I can default to 1 if you'd like, but from experience I wouldn't do that. We might have to come back and readjust it.
This test takes about 11s on a single GPU system, so I don't think it is a big impact.
| logger.info(f"Test Binary: {rocr_debug_agent_test_bin}") | ||
| logger.info(f"Test Script: {rocr_debug_agent_test_script}") | ||
| logger.info(f"Test Bin Dir: {test_bin_dir}") |
There was a problem hiding this comment.
can remove if not used? can't tell if these are debug logs or meant to be here.
if meant to be here, maybe we can combine in one log?
There was a problem hiding this comment.
Since we'll be running these tests constantly with every PR, the output is an important part of the testing script. We (component owners) would like to know exactly what is going on during a test run, so we can track down issues and address them quickly.
The reasoning here is once something fails, we don't get a second chance to run it. So we might as well capture as much information as possible (paths, invocations, timing etc).
Also, as mentioned before, this testing script should also be used by testers/developers.
|
@geomin12 Alternatively, if the testing script is deemed too complex to live in TheRock, I can take it somewhere else and invoke it from TheRock. Does that sound like a better alternative? The functionality would be the same, but we'd need to incorporate the testing script in the project first. |
These should run in as many GPU targets as possible and they tend to run fairly quickly (less than 2 minutes). The testing script tries to be a bit more robust to prevent spurious failures. For instance, if a run fails, it attempts the run the tests 2 more times by default, with an increasing delay between the runs.
- Move argument parsing to the top. - Use Path separator as opposed to explicit slashes.
9e84554 to
7141fc7
Compare
no it should be completely okay here! would rather have all the test scripts live together in one place! |
There was a problem hiding this comment.
i was unaware this may be used for local developers, so the extra arguments and color makes sense!
a lot of failures are occurring today due to github issues, but it works here: https://github.com/ROCm/TheRock/actions/runs/21818081510/job/62953984542?pr=2859
lgtm! thanks for the clarification replies :)
|
Thanks for the review! |
|
@lumachad |
|
Filed #3373. |
Enable CI tests for the rocr-debug-agent component.
These should run in as many GPU targets as possible and they tend to run fairly quickly (less than 2 minutes).
The testing script tries to be a bit more robust to prevent spurious failures. For instance, if a run fails, it attempts to run the tests 2 more times by default, with an increasing delay between the runs.