[Issue]: catch_discover_tests_compile_time_detection does not work

[pvelesko]

Problem Description

This function is supposed to perform test discovery during compilation - this is how it used to work in older versions of HIP (while tests were still part of that repo).

Without this functionality we have to run the test discovery at runtime which in our case takes 70s. So to run a single 1 second long test, it takes us 71s which hinders development.

Additionally, even when using the runtime discovery path, the discovered tests are not being cached which is also a regress from the old HIP.

Operating System

Ubuntu

CPU

Intel

GPU

Intel Arc

ROCm Version

None

ROCm Component

No response

Steps to Reproduce

No response

(Optional for Linux users) Output of /opt/rocm/bin/rocminfo --support

No response

Additional Information

No response

[pvelesko]

Fixed by importing the old CatchAddTests.cmake in #459

[tcgu-amd]

@pvelesko Thanks for reporting the issue. While your PR is still in the queue internally, I am wondering for this particular issue, would using catch2 be a potential solution? Thanks!

[pvelesko]

@tcgu-amd this is using catch2, no?

[tcgu-amd]

@pvelesko Sorry about the confusion, what I meant to say is have you tried to manually use the function defined here, as I wasn't sure the context of this issue, whether the function broken, missing, or not performing expectedly?

Also, apologies for the late response, but it seems like there were multiple build errors for you pull request in our internal testing environment. As a result, I don't think it will get merged anytime soon. Would you be able to confirm which part of it should fix this particular issue? Maybe we can break it down and get this sorted out first.

Thanks :D

[pvelesko]

manually use the function defined

First time seeing this function. Seems like it should work.

Also, apologies for the late response, but it seems like there were multiple build errors for you pull request in our internal testing environment

I opened an issue o March 11 so if you are trying to merge now I am not surprised that there are conflicts.

Maybe we can break it down and get this sorted out first.

I've opened several PRs with AMD and every time we run into a situation like this: by the time someone looks at it things are already conflicting, I am asked to rebase, nobody looks at the rebase, repeat the process.

Case in point: #459

May 6 someone said they are looking into it and will get back to me soon. It's been been 4 months since that comment.

[tcgu-amd]

I opened an issue o March 11 so if you are trying to merge now I am not surprised that there are conflicts.

Maybe we can break it down and get this sorted out first.

I've opened several PRs with AMD and every time we run into a situation like this: by the time someone looks at it things are already conflicting, I am asked to rebase, nobody looks at the rebase, repeat the process.

Case in point: #459

May 6 someone said they are looking into it and will get back to me soon. It's been been 4 months since that comment.

Thank you so much for continuously being active and contributing to ROCm. We understand your frustration; as a result, we have recently established a dedicated team for addressing issues on Github. Starting from now on, we will try our best to hopefully ensure issues are being addressed in a timely fashion.

In this case, I think your PR was in the system back in May when build errors were encountered. We apologize for the lack of follow-ups, and we will try to keep you posted for future updates.

For future PR's, it could help to break down changes into smaller portions to help accelerate the merge process. It would make it easier for us to review, approve, and modify the code internally. When a PR is too big, chances are it will get stuck somewhere in the system due to the approvals and changes it requires.

[tcgu-amd]

First time seeing this function. Seems like it should work.

Cool! Would you mind if I close the issue then? As for your PR, we can leave it open but I am not sure if it will be merged in the near future. Please feel free it break it down to smaller PRs and we will try to see if we can incorporate the changes incrementally. Thank you!