Skip to content

Conversation

@tbqh
Copy link
Collaborator

@tbqh tbqh commented Sep 4, 2025

Implements recommendation from #3020. Adds runAndValidate() to wrap the scheduleAndRun() and testValidate() pattern that is common in many tests.

IMO I'm not a huge fan of this change. There is a lot of variety in the way scheduleAndRun() and testValidate() are called. scheduleAndRun() has a default argument, and testValidate() has 3 overloads, some of which also have default arguments. I opted not to add any overloads for runAndValidate(), which means that there's plenty of cases where it can't be used. I think it was a little confusing the number of patterns we had before, and this PR makes it worse by adding another one.

And due to 80col limit, this PR is more verbose in some cases.

@tbqh tbqh requested a review from csarofeen September 4, 2025 16:11
@github-actions
Copy link

github-actions bot commented Sep 4, 2025

Description

  • Introduce runAndValidate utility function

  • Replace scheduleAndRun and testValidate calls

  • Simplify test validation across multiple files

  • Improve code consistency and maintainability


Changes walkthrough 📝

Relevant files
Enhancement
9 files
test_gpu2.cpp
Replace scheduleAndRun and testValidate with runAndValidate
+16/-20 
test_gpu3.cpp
Replace scheduleAndRun and testValidate with runAndValidate
+4/-6     
test_indexing_advanced.cpp
Replace scheduleAndRun and testValidate with runAndValidate
+4/-6     
test_pointwise.cpp
Replace scheduleAndRun and testValidate with runAndValidate
+12/-14 
test_reshape.cpp
Replace scheduleAndRun and testValidate with runAndValidate
+22/-33 
test_resize.cpp
Replace scheduleAndRun and testValidate with runAndValidate
+9/-17   
test_transpose.cpp
Replace scheduleAndRun and testValidate with runAndValidate
+26/-31 
utils.cpp
Add runAndValidate utility function implementation             
+12/-0   
utils.h
Declare runAndValidate utility function                                   
+7/-0     

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
⚡ No major issues detected

@tbqh tbqh requested a review from naoyam September 4, 2025 17:06
@tbqh tbqh changed the title Run and validate Add runAndValidate() to wrap scheduleAndRun() and testValidate() pattern Sep 4, 2025
@naoyam
Copy link
Collaborator

naoyam commented Sep 5, 2025

!test

Copy link
Collaborator

@naoyam naoyam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I'm not that worried about the multiple overloaded versions as these are only test utilities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants